Skip to content

OTelトレースを計装する#2577

Merged
ne-sachirou merged 4 commits intomainfrom
opentelemetry-exporter-otlp
Sep 23, 2025
Merged

OTelトレースを計装する#2577
ne-sachirou merged 4 commits intomainfrom
opentelemetry-exporter-otlp

Conversation

@ne-sachirou
Copy link
Member

@ne-sachirou ne-sachirou commented Jul 10, 2025

dreamkastにOpenTelemetryトレースを計装する。
一般的な設定例はトレース - RubyにOpenTelemetryを導入する - Mackerel ヘルプ

手元から送ってみたトレースの様子 : https://cloudnativedays.slack.com/archives/C02MAP9EHAA/p1758219358193209

OpenTelemetryコレクターはサイドカーコンテナとして同じECSタスク内に立っている。トレースはコレクターに送り、コレクターがバックエンド(Mackerel)に送る。サンプリングなどの処理はコレクターで行う。コレクターの設定は cloudnativedaysjp/dreamkast-infra#4713 の通り。

@ne-sachirou ne-sachirou self-assigned this Jul 10, 2025
@gitops-for-cloudnativedays gitops-for-cloudnativedays bot added the reviewapps Build ReviewApp environment automatically if this label is granted label Jul 10, 2025
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 10, 2025
commit: cloudnativedaysjp/dreamkast@6c037d8
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16193757110

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@ne-sachirou ne-sachirou changed the title gem OTelトレースを計装する Jul 10, 2025
@github-actions github-actions bot removed the reviewapps Build ReviewApp environment automatically if this label is granted label Aug 9, 2025
@ne-sachirou ne-sachirou force-pushed the opentelemetry-exporter-otlp branch from 6c037d8 to febb694 Compare September 17, 2025 07:07
@gitops-for-cloudnativedays gitops-for-cloudnativedays bot added the reviewapps Build ReviewApp environment automatically if this label is granted label Sep 17, 2025
@claude
Copy link

claude bot commented Sep 17, 2025

Claude finished @ne-sachirou's task —— View job


コードレビューの結果

このPRでOpenTelemetryトレースの計装を実装されていますね。全体的によく構造化された実装になっていると思います。以下、詳細なフィードバックをお伝えします。

✅ 良い点

  1. ベストプラクティスに従った実装

    • 標準的なOpenTelemetry SDKの使い方に準拠しています
    • 環境変数による設定の切り替えができるようになっています
    • BatchSpanProcessorを使用しており、パフォーマンスを考慮した設計です
  2. 適切なエラーハンドリング

    • ApplicationControllerで例外をトレースに記録する実装が追加されています
    • render_500メソッドでも例外情報を記録しています
  3. 開発環境での配慮

    • OTEL_ENDPOINTが未設定の場合はコンソール出力に切り替える実装
    • docker-compose.ymlで開発環境用の設定が追加されています

🔍 改善提案

  1. ログレベルの調整

    • config/initializers/otel.rb:23行目のログレベルはすでにレビューで指摘されているようにwarnが適切です
  2. 設定値の外部化を検討

    c.service_version = '1.0.0'  # config/initializers/otel.rb:8
    • バージョンはハードコーディングではなく、環境変数や定数から取得することを推奨します
    • 例:ENV['APP_VERSION'] || '1.0.0'
  3. HTTPヘッダーの設定

    headers: {
      'Accept' => '*/*'
    }
    • 通常OTLPエクスポーターではContent-Typeの設定が必要な場合があります
    • コレクター側の要件を確認してください

⚠️ 潜在的な問題

  1. エラー記録の重複

    • ApplicationController:14行目と91行目で同じ例外を2回記録している可能性があります
    • render_500が呼ばれる前にすでに記録されているケースでは重複になります
  2. development環境でのrescue_from

    • unless Rails.env.development?の条件により、開発環境では例外のトレース記録が行われません
    • 開発環境でもトレースを確認したい場合は、この条件を見直す必要があるかもしれません

🚀 パフォーマンスの考慮事項

  1. バッチ処理の設定

    • BatchSpanProcessorのデフォルト設定(バッチサイズ、タイムアウト等)で問題ないか確認してください
    • 高トラフィックな環境では調整が必要な場合があります
  2. サンプリング戦略

    • 現在の実装では全てのトレースを送信する設定になっています
    • 本番環境では適切なサンプリング戦略の実装を検討してください

🔒 セキュリティの懸念事項

特に重大なセキュリティ上の問題は見当たりませんでした。OpenTelemetryの実装は適切に行われています。

🧪 テストカバレッジ

OpenTelemetryの設定に関するテストが見当たりませんでした。以下のようなテストの追加を検討してください:

  1. 環境変数の有無による動作の違いを確認するテスト
  2. 例外発生時にトレースに記録されることを確認するテスト
  3. 初期化処理が正常に完了することを確認するテスト

📝 その他の提案

  1. ドキュメントの追加

    • READMEや開発者向けドキュメントにOpenTelemetryの設定方法を記載することを推奨します
  2. モニタリング

    • トレースが正常に送信されているかを確認する仕組みの導入を検討してください

全体的に良い実装だと思います。上記の点を考慮していただければ、より堅牢な実装になると思います。

gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Sep 17, 2025
commit: cloudnativedaysjp/dreamkast@febb694
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/17789625558

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Sep 18, 2025
commit: cloudnativedaysjp/dreamkast@2c86f87
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/17836627993

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@ne-sachirou ne-sachirou force-pushed the opentelemetry-exporter-otlp branch from 2c86f87 to 84da47a Compare September 18, 2025 18:10
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Sep 18, 2025
commit: cloudnativedaysjp/dreamkast@84da47a
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/17837510111

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@ne-sachirou ne-sachirou marked this pull request as ready for review September 18, 2025 18:27
@ne-sachirou ne-sachirou requested a review from a team September 19, 2025 03:38
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Sep 19, 2025
commit: cloudnativedaysjp/dreamkast@6071fc3
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/17848206248

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@github-actions
Copy link

Simplecov Report

Covered Threshold
64.38% 60%

Co-authored-by: Ryo Takaishi <ryo.takaishi.0@gmail.com>
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Sep 23, 2025
commit: cloudnativedaysjp/dreamkast@384ebad
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/17952896746

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@github-actions
Copy link

@ne-sachirou ne-sachirou merged commit 777c88e into main Sep 23, 2025
10 checks passed
@ne-sachirou ne-sachirou deleted the opentelemetry-exporter-otlp branch September 23, 2025 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewapps Build ReviewApp environment automatically if this label is granted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants