Add category-specific Prometheus metrics to DreamkastExporter

[b1gb4by]

Summary

• CND/PEK/SREKカテゴリ別に7種類のPrometheusメトリクスを追加
  • dreamkast_talk_difficulties_by_category_count (既存)
  • dreamkast_assumed_visitors_by_category_count (想定受講者)
  • dreamkast_execution_phases_by_category_count (実行フェーズ)
  • dreamkast_publication_permissions_by_category_count (公開許可)
  • dreamkast_session_times_by_category_count (セッション時間)
  • dreamkast_languages_by_category_count (言語)
  • dreamkast_proposals_by_category_count (プロポーザル数)
• 未使用の重いメトリクス(dreamkast_select_talks, dreamkast_select_proposal_items)を無効化
• リクエスト内キャッシュと共通ヘルパーメソッドによるクエリ最適化

Changes

• app/middlewares/dreamkast_exporter.rb: 6つの新メトリクス定義・集計メソッド・3つのヘルパーメソッド追加、未使用メトリクス2つをコメントアウト
• spec/requests/exporter_spec.rb: 新メトリクス6項目分のテストケース追加(ProposalItemConfig/ProposalItemのテストデータ含む)

Test plan

• bundle exec rspec spec/requests/exporter_spec.rb 全10テストパス
• bundle exec rubocop app/middlewares/dreamkast_exporter.rb 違反なし
• ローカルサーバーで /metrics エンドポイント確認済み
• ステージング環境で実データでのメトリクス出力を確認

🤖 Generated with Claude Code

[b1gb4by]

@claude レビューをお願いします。

[claude]

Claude finished @b1gb4by's task in 1m 15s —— View job

---

レビュー進捗

• 変更ファイルの読み込み
• コード実装のレビュー
• テストコードのレビュー
• レビューフィードバックの提供

---

レビューサマリー

全体的によく実装されており、コードの品質も高いです。以下のような良い点が見られます:

良い点:

• 6つの新メトリクスの追加と未使用メトリクスの無効化により、パフォーマンス改善が見込めます
• リクエスト内キャッシュ(@category_talk_ids_map, @proposal_item_config_params_map)による最適化が適切です
• count_by_categoryヘルパーメソッドで共通ロジックをDRYに実装しています
• テストカバレッジが充実しており、エッジケース(複数カテゴリに属するトーク)もテストされています

指摘事項

1. 🟡 中: N+1クエリの可能性 (count_by_category)

app/middlewares/dreamkast_exporter.rb:321

count_by_categoryメソッドでカテゴリごとにProposalItem.whereを実行しているため、カテゴリ数分のクエリが発生します:

category_talk_ids_map.each do |category, talk_ids|
  items = ProposalItem.where(talk_id: talk_ids, label:, conference_id: CONFERENCE_ID)
  # ...
end

推奨改善案:

def count_by_category(metrics, label:, value_label_name:, is_checkbox:)
  config_map = proposal_item_config_params_map
  
  # 全カテゴリのtalk_idを1つのクエリで取得
  all_talk_ids = category_talk_ids_map.values.flatten.uniq
  items = ProposalItem.where(talk_id: all_talk_ids, label:, conference_id: CONFERENCE_ID)
  
  # talk_id -> category のマッピングを作成
  talk_id_to_categories = category_talk_ids_map.each_with_object({}) do |(category, talk_ids), hash|
    talk_ids.each { |tid| (hash[tid] ||= []) << category }
  end
  
  # カテゴリごとにグループ化してカウント
  counts = Hash.new { |h, k| h[k] = Hash.new(0) }
  items.each do |item|
    param_ids = is_checkbox ? Array(item.params) : [item.params]
    param_ids.compact.each do |pid|
      name = config_map[pid.to_i]
      next unless name
      
      talk_id_to_categories[item.talk_id]&.each do |category|
        counts[category][name] += 1
      end
    end
  end
  
  counts.each do |category, name_counts|
    name_counts.each do |name, count|
      metrics.set(
        count,
        labels: {
          conference_id: CONFERENCE_ID,
          target_conference: category,
          value_label_name.to_sym => name
        }
      )
    end
  end
end

この変更により、カテゴリ数(3つ)×メトリクス数(4つ)=12クエリが1クエリに削減されます。

2. 🟡 中: dreamkast_assumed_visitors_by_category_countのカテゴリロジック不整合

app/middlewares/dreamkast_exporter.rb:233-266

このメソッドはlabel_to_categoryマッピングを使用していますが、他のカテゴリベースメトリクスはcategory_talk_ids_mapを使用しています。これにより、assumed_visitorの集計方法が異なります:

• dreamkast_assumed_visitors_by_category_count: assumed_visitorのラベル(cnd_assumed_visitorなど)からカテゴリを推論
• 他のメトリクス: カテゴリラベル(cnd_categoryなど)に属するtalk_idから集計

問題点:

• トークがcategoryに属していても、対応するassumed_visitorラベルがない場合、カウントされない
• 逆に、assumed_visitorラベルがあってもcategoryラベルがない場合もカウントされる可能性

推奨改善案:

def dreamkast_assumed_visitors_by_category_count(metrics)
  config_map = proposal_item_config_params_map
  
  # カテゴリとassumed_visitorラベルのマッピング
  category_to_av_label = {
    'cnd_category' => 'cnd_assumed_visitor',
    'pek_category' => 'pek_assumed_visitor',
    'srek_category' => 'srek_assumed_visitor'
  }
  
  category_talk_ids_map.each do |category, talk_ids|
    av_label = category_to_av_label[category]
    next unless av_label
    
    items = ProposalItem.where(talk_id: talk_ids, label: av_label, conference_id: CONFERENCE_ID)
    counts = Hash.new(0)
    
    items.each do |item|
      Array(item.params).compact.each do |pid|
        name = config_map[pid.to_i]
        counts[name] += 1 if name
      end
    end
    
    counts.each do |name, count|
      metrics.set(
        count,
        labels: {
          conference_id: CONFERENCE_ID,
          target_conference: category,
          assumed_visitor_name: name
        }
      )
    end
  end
end

これにより、他のメトリクスと一貫した集計ロジックになります。

3. 🟢 軽微: テストデータの一貫性

spec/requests/exporter_spec.rb:90-92

テストではassumed_visitorをcategoryに紐づくtalk_idにしか設定していませんが、現在の実装ではcategoryラベルなしでもassumed_visitorラベルがあればカウントされます。テストが実装の意図を正確に表しているか確認してください。

4. 🟢 軽微: マジックナンバーのハードコーディング

app/middlewares/dreamkast_exporter.rb:5

CONFERENCE_ID = 15

このIDがクラウドネイティブ会議固有のものであることはコメントで説明されていますが、環境変数や設定ファイルからの取得も検討する価値があります。将来的に他の会議でも同様のメトリクスが必要になる可能性がある場合は柔軟性が高まります。

5. 🟢 軽微: コメントアウトコードの扱い

app/middlewares/dreamkast_exporter.rb:40-50, 173-190

コメントアウトされた2つの未使用メトリクスは、PRの説明通り意図的に無効化されています。ただし、将来的に完全に削除する予定があるのか、一時的な無効化なのかをコメントで明記すると良いでしょう:

# カテゴリ別メトリクスで代替されたため無効化(将来的に削除予定)
# 注意: 再度有効化する場合はパフォーマンスへの影響を評価すること

テストコードについて

テストは非常に充実しており、以下のケースがカバーされています:

• ✅ 各新メトリクスの基本動作
• ✅ 複数カテゴリに属するトークのケース(line 121-136)
• ✅ CheckBox(配列)とRadioButton(文字列)の両方のparamsタイプ
• ✅ ProposalItemConfigとの結合テスト

特に複数カテゴリのテストケースは重要なエッジケースをカバーしており、素晴らしいです。

総合評価

実装は全体的に良好で、テストも充実しています。主な改善提案は:

1. 必須: N+1クエリの最適化(指摘1)
2. 推奨: assumed_visitorsの集計ロジックを他のメトリクスと統一(指摘2)

これらを修正すれば、パフォーマンスと保守性がさらに向上します。指摘1のN+1クエリ問題は、カテゴリ数が少ない(3つ)ため現時点では大きな影響はありませんが、将来的なスケーラビリティのために修正することを推奨します。

---

[github-actions]

Review app

• https://dreamkast-dk-2727.dev.cloudnativedays.jp

[github-actions]

Simplecov Report

Covered	Threshold
65.74%	60%

[b1gb4by]

@claude 再度レビューをお願いします。

[claude]

Claude finished @b1gb4by's task in 1m 24s —— View job

---

レビュー進捗

• 変更ファイルの読み込み
• コード実装のレビュー
• テストコードのレビュー
• レビューフィードバックの提供

---

レビューサマリー

前回のレビューで指摘した主要な問題が適切に修正されており、コードの品質が大幅に向上しています。特に以下の改善が確認できました:

前回からの主な改善点:

• ✅ N+1クエリ問題の解決: count_by_categoryメソッドが全面的に書き直され、全カテゴリのtalk_idを1クエリで取得する実装に変更されました(L331-332)
• ✅ assumed_visitorsロジックの統一: 他のメトリクスと同様にcategory_talk_ids_mapを使用する実装に変更され、カテゴリロジックが一貫しています(L243-268)
• ✅ コメントアウトコードへのコメント追加: 無効化されたメトリクスに理由が明記されています(L40, L173)

現在の実装の良い点

1. パフォーマンス最適化が優れている

   • リクエスト内キャッシュ(@category_talk_ids_map, @proposal_item_config_params_map)により、複数メトリクス間でクエリ結果を再利用
   • count_by_categoryメソッドで全カテゴリのデータを1クエリで取得(L331-332)
   • dreamkast_talk_difficulties_by_category_countではjoins + groupを使った効率的な集計(L216-219)

2. コードの保守性が高い

   • count_by_categoryヘルパーメソッドで共通ロジックをDRY化(L320-360)
   • category_talk_ids_mapとproposal_item_config_params_mapでデータアクセスを一元化
   • CheckBox(配列)とRadioButton(文字列)の違いをis_checkboxパラメータで抽象化

3. テストカバレッジが充実

   • 全6つの新メトリクスに対するテストケース
   • 複数カテゴリに属するトークのエッジケース(L121-136)
   • CheckBoxとRadioButtonの両タイプをカバー

指摘事項

1. 🟢 軽微: respond_withメソッドでのキャッシュクリア位置

app/middlewares/dreamkast_exporter.rb:106-113

キャッシュ変数のクリアがメトリクス収集の前に行われていますが、これらの変数はsend(metrics.name, metrics)内で初期化されるため、現在の位置は問題ありません。ただし、より明示的にするなら以下のようにコメントを追加することを検討してください:

def respond_with(format)
  # リクエストごとにキャッシュをクリア(各メトリクスメソッド内で遅延初期化される)
  @category_talk_ids_map = nil
  @proposal_item_config_params_map = nil
  @registry.metrics.each do |metrics|
    send(metrics.name, metrics)
  end
  super
end

2. 🟢 軽微: count_by_categoryの変数名をより明確に

app/middlewares/dreamkast_exporter.rb:322

cat_mapという短縮形よりもcategory_mapの方が可読性が高いです:

def count_by_category(metrics, label:, value_label_name:, is_checkbox:)
  config_map = proposal_item_config_params_map
  category_map = category_talk_ids_map  # cat_map -> category_map

  # talk_id -> 所属カテゴリ一覧のマッピング
  talk_id_to_categories = {}
  category_map.each do |category, talk_ids|
    talk_ids.each { |tid| (talk_id_to_categories[tid] ||= []) << category }
  end

  # 全カテゴリのtalk_idをまとめて1クエリで取得
  all_talk_ids = category_map.values.flatten.uniq
  # ...
end

3. 🟢 軽微: ProposalItemConfigのtypeフィールドについて

spec/requests/exporter_spec.rb:72-87

テストでProposalItemConfigを作成する際、type: 'ProposalItemConfigCheckBox'やtype: 'ProposalItemConfigRadioButton'を指定していますが、実装側ではこのtypeフィールドを使用していません。is_checkboxパラメータで判断しているため、テストデータとしてtypeを指定する必要性が不明確です。

質問: このtypeフィールドは別の用途で使用されているのでしょうか？もしテスト専用であれば、コメントで説明を追加するか、不要であれば削除を検討してください。

4. 🟢 情報: 複数カテゴリに属するトークの扱い

現在の実装では、1つのトークが複数のカテゴリに属する場合、各カテゴリで個別にカウントされます(L121-136のテストで確認済み)。これは以下のメトリクスで一貫しています:

• dreamkast_talk_difficulties_by_category_count: JOIN時に各カテゴリで個別にカウント(L216-219)
• dreamkast_assumed_visitors_by_category_count: カテゴリごとのループで個別にカウント(L243-268)
• その他のcount_by_categoryメソッド使用メトリクス: talk_id_to_categoriesマッピングで各カテゴリにカウント(L342-344)

この仕様は正しく実装されており、テストでも検証されています。ドキュメントやコメントで明記されているため、問題ありません。

テストコードについて

テストは非常に充実しており、以下のケースがカバーされています:

• ✅ 各新メトリクスの基本動作(L110-185)
• ✅ 複数カテゴリに属するトークのエッジケース(L121-136)
• ✅ CheckBox(配列)とRadioButton(文字列)の両方のparamsタイプ
• ✅ ProposalItemConfigとの結合テスト

特にL121-136の複数カテゴリテストは、CNDとPEKの両方にカウントされることを明示的に検証しており、素晴らしいです。

総合評価

前回の必須改善事項がすべて適切に対応されており、コードの品質は本番環境に投入可能なレベルです。

主な成果:

1. ✅ N+1クエリ問題の完全な解決
2. ✅ カテゴリロジックの統一と一貫性の向上
3. ✅ パフォーマンス最適化の実装
4. ✅ 充実したテストカバレッジ

現在の指摘事項はすべて軽微なもので、オプショナルな改善提案です。現状のままでもマージして問題ありません。

指摘3のProposalItemConfigのtypeフィールドについては確認事項ですが、実装上の問題ではありません。

推奨アクション: マージ可能です。指摘1-2の軽微な改善は、今後のコードメンテナンス時に対応しても問題ありません。

---