10 Grafanaのエンジニアリングチームから学んだコード品質ルール

はじめに

Grafanaは、7万以上のGitHubスターを獲得し、数千人の貢献者が日々改善している、最も人気のあるオープンソースの可観測性プラットフォームの1つです。3千以上の未解決の課題と数百のプルリクエストが常に進行しているため、コードベースをクリーンで一貫性のある状態に保つことは真の課題です。

コードベースを研究することで、チームが迅速に動きながら品質を高く保つのに役立つ、暗黙のルールやベストプラクティスを明らかにできます。これらのルールの多くは、セキュリティ、保守性、信頼性に焦点を当てています。その中には、不適切な非同期使用、リソースリーク、コード全体の一貫性のないパターンなど、従来の静的解析ツール（SAST）では検出できない問題もあります。これらは、人間のレビュアーやAI搭載ツールがコードレビュー中に発見できる種類の問題です。

課題

このような大規模プロジェクトは、いくつかの課題に直面します。膨大なコード量、多数のモジュール（API、UI、プラグイン）、そして無数の外部連携（Prometheus、Lokiなど）です。何百人ものコントリビューターが異なるコーディングスタイルや前提に従う可能性があります。新機能や迅速な修正が、隠れたバグ、セキュリティ上の欠陥、または分かりにくいコードパスを導入することがあります。ボランティアのレビュー担当者は、コードベースのすべての部分を把握しているわけではないため、設計パターンやベストプラクティスを見落とす可能性があります。要するに、貢献の規模と多様性が、一貫性と信頼性の維持を困難にしています。

これらの規則が重要な理由

明確なレビュー規則は、Grafanaの健全性に直接的な利益をもたらします。まず、保守性が向上します。一貫したパターン（フォルダーレイアウト、命名、エラー処理）により、コードの読み取り、テスト、拡張が容易になります。誰もが共通の慣例に従うことで、レビュー担当者は意図を推測する時間を減らすことができます。次に、セキュリティが強化されます。「常にユーザー入力を検証する」や「オープンリダイレクトを避ける」といった規則は、Grafanaで発見された脆弱性（CVE-2025-6023/4123など）を防止します。最後に、新しい貢献者のオンボーディングが迅速になります。例やレビューで一貫して同じプラクティスが使用されることで、新規参入者は「Grafana流」を迅速かつ自信を持って習得できます。

これらのルールに文脈を結びつける

これらのルールは、Grafanaのコードとコミュニティにおける実際の課題から生まれました。セキュリティアドバイザリやバグ報告により、予防ルールへと変換されるパターン（例：XSSにつながるパストラバーサル）が明らかになりました。以下の各ルールでは、具体的な落とし穴を強調し、それが重要である理由（パフォーマンス、明確性、セキュリティなど）を説明し、Grafanaが使用する言語（GoまたはTypeScript/JS）で、❌ 非準拠の例と ✅ 準拠の例を明確なスニペットで示します。

では、Grafanaのコードベースを堅牢で安全、かつ理解しやすい状態に保つための10のルールを見ていきましょう。

‍

10 Grafanaにインスパイアされた実践的なコード品質ルール

1. 設定には環境変数を使用してください（ハードコードされた値は避けてください）。

‍ポート、認証情報、URL、その他の環境固有の値をハードコーディングしないでください。コードの柔軟性を保ち、シークレットがソースコードに含まれないように、環境変数または設定ファイルから読み込んでください。

❌ 非準拠：

// server.js
const appPort = 3000;
app.listen(appPort, () => console.log("Listening on port " + appPort));

✅ 準拠：

// server.ts
const PORT = Number(process.env.PORT) || 3000;
app.listen(PORT, () => console.log(`Listening on port ${PORT}`));

‍これが重要な理由: 環境変数を使用することで、機密データをソースコードから分離し、さまざまな環境でのデプロイを柔軟にし、シークレットの偶発的な漏洩を防ぐことができます。また、設定変更にコードの修正が不要になるため、保守性が向上し、エラーが削減されます。

2. ユーザー入力を利用する前にサニタイズする。‍

インジェクション攻撃や予期せぬ動作を防ぐため、ユーザーまたは外部ソースからのすべての入力は使用前に検証またはサニタイズされるべきです。

❌ 非準拠：

// frontend/src/components/UserForm.tsx
const handleSubmit = (username: string) => {
  setUsers([...users, { name: username }]);
};

✅ 準拠：

// frontend/src/utils/sanitize.ts
export function sanitizeInput(input: string): string {
  return input.replace(/<[^>]*>/g, ''); // removes HTML tags
}

// frontend/src/components/UserForm.tsx
import { sanitizeInput } from '../utils/sanitize';

const handleSubmit = (username: string) => {
  const cleanName = sanitizeInput(username);
  setUsers([...users, { name: cleanName }]);
};

これが問題となる理由: 適切な入力サニタイズは、XSS、インジェクション攻撃、および不正な入力によって引き起こされる予期しない動作を防ぎます。これにより、ユーザーとシステムの両方が保護され、ダウンストリームプロセス、ロギング、およびストレージがデータを安全に処理することが保証されます。

3. オープンリダイレクトとパストラバーサルを防止します。

コードで使用されるURLまたはファイルパスが適切に検証およびサニタイズされていることを確認してください。ユーザー入力がリダイレクトやファイルシステムパスを直接決定することを許可しないでください。

❌ 非準拠：

// Express route in Grafana plugin
app.get("/goto", (req, res) => {
  const dest = req.query.next;    // attacker can supply any URL
  res.redirect(dest);
});

✅ 準拠：

// Express route with safe redirect
app.get("/goto", (req, res) => {
  const dest = req.query.next;
  // Only allow relative paths starting with '/'
  if (dest && dest.startsWith("/")) {
    res.redirect(dest);
  } else {
    res.status(400).send("Invalid redirect URL");
  }
});

これが問題となる理由: オープンリダイレクトとパストラバーサルを防ぐことは、ユーザーをフィッシング、データ漏洩、不正なファイルアクセスから保護します。これにより、攻撃対象領域が低減され、セキュリティ境界が強制され、機密性の高いサーバーリソースの偶発的な公開が回避されます。

4. 厳格なコンテンツセキュリティポリシー（CSP）を有効にします。

アプリケーションヘッダーでコンテンツセキュリティポリシーを適用し、信頼できるソースからのスクリプト、スタイル、画像、その他のリソースのみを許可します。unsafe-inline、eval、ワイルドカードソースは許可しません。

❌ 非準拠: (CSPなし、または許可しすぎ)

# grafana.ini (非準拠)
content_security_policy = false

✅ 準拠: (設定で強力なCSPを使用)

# grafana.ini
content_security_policy = true
content_security_policy_template = """
  script-src 'self' 'unsafe-eval' 'unsafe-inline' 'strict-dynamic' $NONCE;
  object-src 'none';
  font-src 'self';
  style-src 'self' 'unsafe-inline' blob:;
  img-src * data:;
  base-uri 'self';
  connect-src 'self' grafana.com ws://$ROOT_PATH wss://$ROOT_PATH;
  manifest-src 'self';
  media-src 'none';
  form-action 'self';
"""

重要な理由：厳格なCSPは、XSSを含む多くの種類のクライアントサイド攻撃をブロックします。これにより、リソースの予測可能な動作が強制され、悪意のあるコード実行の可能性が低減し、ブラウザコンテキストにおける明確なセキュリティ境界が提供されます。

5. エラーとnilチェックを処理します（パニックは避けてください）。

関数呼び出し、API応答、データ構造においては、常にエラーとnil値を確認してください。パニックを適切なエラー処理に置き換え、意味のあるエラーメッセージまたはコードを返すようにしてください。

❌ 非準拠：

rows, _ := db.Query("SELECT * FROM users WHERE id=?", id)  // ignored error
user := &User{}
rows.Next()
rows.Scan(&user.Name)  // rows might be empty => user is nil => panic

✅ 準拠：

rows, err := db.Query("SELECT * FROM users WHERE id=?", id)
if err != nil {
    return nil, err
}
defer rows.Close()
if !rows.Next() {
    return nil, errors.New("user not found")
}
var name string
if err := rows.Scan(&name); err != nil {
    return nil, err
}
user := &User{Name: name}

これが問題となる理由: 適切なエラー処理はクラッシュを防ぎ、予期しない入力や条件が発生した場合でもシステムが信頼性を維持することを保証します。これにより、保守性が向上し、ダウンタイムが削減され、意味のあるエラー情報を提供することでデバッグが容易になります。

6. リソースのクリーンアップを遅延させる（リーク防止）。

ファイル、ネットワーク接続、データベースハンドルなどの開かれたすべてのリソースが、割り当て直後にdeferを使用して適切に閉じられるようにしてください。コードの後半での手動クリーンアップに依存しないでください。

❌ 非準拠：

resp, err := http.Get(url)
// ... use resp.Body ...
// forgot: resp.Body.Close()

✅ 準拠：

resp, err := http.Get(url)
if err != nil {
    // handle error
}
defer resp.Body.Close()
// ... use resp.Body ...

これが問題となる理由: 適切なクリーンアップは、メモリリーク、ファイルディスクリプタの枯渇、およびコネクションプールの飽和を防ぎます。これにより、システム安定性が維持され、時間経過によるパフォーマンス低下が回避され、本番環境での運用上の問題が低減されます。

7. パラメータ化クエリを使用する（SQLインジェクションの回避）。

SQLコマンドに文字列連結を使用する代わりに、データベースとのやり取りには常にパラメーター化クエリまたはプリペアドステートメントを使用してください。

❌ 非準拠：

// 危険: userIDにSQLクォートまたはインジェクションが含まれる可能性があります
query := "DELETE FROM sessions WHERE user_id = '" + userID + "';"
db.Exec(query)

✅ 準拠：

// 安全: userIDはパラメータとして渡されます
db.Exec("DELETE FROM sessions WHERE user_id = ?", userID)

これが問題となる理由: パラメータ化されたクエリは、最も一般的なセキュリティ脆弱性の一つであるSQLインジェクション攻撃を防ぎます。これにより、機密データが保護され、データベース破損のリスクが低減され、クエリの保守性が高まり、監査が容易になります。これは、アプリケーションのセキュリティと信頼性の両方を確保します。

8. TypeScriptでasync/awaitを適切に使用する（プロミスの処理）。

‍リジェクションを無視したり、コールバック形式の処理を混在させたりするのではなく、プロミスは常にawaitし、try/catchを使用してエラーを処理します。

❌ 非準拠：

async function fetchData() {
  // Missing await: fetch returns a Promise, not the actual data
  const res = fetch('/api/values');
  console.log(res.data); // undefined
}

✅ 準拠：

async function fetchData() {
  try {
    const res = await fetch('/api/values');
    const data = await res.json();
    console.log(data);
  } catch (err) {
    console.error("Fetch failed:", err);
  }
}

これが問題となる理由: 適切な非同期処理は、非同期コードのエラーが見過ごされないようにし、未処理のPromiseリジェクションを防ぎ、予測可能なプログラムフローを維持します。これにより、コードがより読みやすく、デバッグしやすくなり、データ破損、一貫性のない状態、または予期しないランタイムクラッシュにつながる可能性のある微妙なバグを防ぎます。

9. TypeScriptでは厳密な型を優先する (anyの使用を避ける)。

変数、関数パラメータ、戻り値の型を定義する際には、anyの代わりに正確なTypeScript型を使用してください。

❌ 非準拠：

// No types specified
function updateUser(data) {
  // ...
}
let config: any = loadConfig();

✅ 準拠：

interface User { id: number; name: string; }
function updateUser(data: User): Promise<User> {
  // ...
}
interface AppConfig { endpoint: string; timeoutMs: number; }
const config: AppConfig = loadConfig();

これが問題となる理由: 厳密な型付けは、コンパイル時に型関連の誤りを捕捉し、ランタイムエラーを削減し、コードの信頼性を向上させます。これにより、コードは自己文書化され、リファクタリングが容易になり、システムのすべての部分が予測可能で型安全な方法で相互作用することが保証されます。これは、Grafanaのような大規模で複雑なコードベースでは非常に重要です。

10. 一貫性のあるコードスタイルと命名規則を適用してください。

コードベース全体で、一貫したフォーマット、命名規則、およびファイル構造を適用します。

❌ 非準拠: (混在するスタイル)

const ApiData = await getdata();   // PascalCase for variable? function name not camelCase.
function Fetch_User() { ... }      // Unusual naming.

✅ 準拠：

const apiData = await fetchData();
function fetchUser() { ... }

重要な理由：一貫したスタイルと命名規則は、可読性を向上させ、複数の貢献者がコードを理解し、保守しやすくします。これにより、プロジェクトをナビゲートする際の認知負荷が軽減され、誤解による微妙なバグが防止され、自動化ツール（リンター、フォーマッター、コードレビューア）が大規模なチーム環境で品質基準を確実に適用できるようになります。

まとめ

上記の各ルールは、Grafanaのコードベースにおける繰り返しの課題に対処しています。コードレビュー中にこれらを一貫して適用することで、チームはクリーンで予測可能なコードを維持し、一般的な脆弱性を防ぐことでセキュリティを向上させ、新規貢献者向けに明確なパターンを提供することでオンボーディングを円滑にします。プロジェクトが拡大するにつれて、これらのプラクティスはコードベースを信頼性が高く、保守可能で、関係者全員にとってナビゲートしやすいものに保ちます。これらのルールに従うことで、あらゆるエンジニアリングチームが大規模な高品質ソフトウェアを構築し、維持するのに役立ちます。

‍

‍

‍