Extend telemetry demo to include logs and traces#73
Merged
Conversation
📝 WalkthroughWalkthroughThe PR expands the observability stack from OpenTelemetry + Prometheus + Grafana to include Promtail, Loki, and Tempo for logs and traces. Configuration files are added for the new services, docker-compose is updated to orchestrate them, documentation is revised to reflect the new stack, and minor GitHub Actions workflow updates are applied. Changes
Sequence DiagramsequenceDiagram
actor App as scanoss-api
participant Promtail as Promtail<br/>(Log Scraper)
participant Loki as Loki<br/>(Log Aggregation)
participant OTEL as OTEL-Collector
participant Tempo as Tempo<br/>(Trace Backend)
participant Prometheus as Prometheus<br/>(Metrics)
participant Grafana as Grafana<br/>(Visualization)
rect rgba(100, 150, 200, 0.5)
Note over App,Prometheus: Telemetry Collection Flow
App->>App: Write logs & traces to stderr/file
Promtail->>App: Scrape logs from /var/log/scanoss/api/*.log
Promtail->>Loki: Push logs via HTTP
Loki->>Loki: Store & index logs
App->>OTEL: Send traces & metrics via OTLP
OTEL->>Tempo: Forward traces to tempo:4317
Tempo->>Tempo: Store traces (72h retention)
OTEL->>Prometheus: Forward metrics
Prometheus->>Prometheus: Store metrics
end
rect rgba(100, 200, 100, 0.5)
Note over Grafana: Visualization & Analysis
Grafana->>Prometheus: Query metrics (exemplars linked)
Prometheus->>Grafana: Metrics + trace_id exemplars
Grafana->>Tempo: Click exemplar → fetch trace
Grafana->>Loki: Query logs via TraceQL or trace_id
Loki->>Grafana: Display related log entries
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
telemetry-demo/docker-compose.yml (1)
61-67:⚠️ Potential issue | 🟠 MajorAdd
--enable-feature=exemplar-storageto enable Prometheus exemplar support.The
grafana-datasources.yamlconfiguresexemplarTraceIdDestinationsto link Prometheus metrics to Tempo traces via exemplars. However, this feature requires the--enable-feature=exemplar-storageflag to be set on Prometheus. Without it, Prometheus won't store exemplars and the trace linking will silently produce no results.🔧 Proposed fix
command: - '--config.file=/etc/prometheus/prometheus.yml' - '--storage.tsdb.path=/prometheus' - '--web.console.libraries=/etc/prometheus/console_libraries' - '--web.console.templates=/etc/prometheus/consoles' - '--storage.tsdb.retention.time=200h' - '--web.enable-lifecycle' + - '--enable-feature=exemplar-storage'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telemetry-demo/docker-compose.yml` around lines 61 - 67, The Prometheus container command block is missing the feature flag to store exemplars; add the flag --enable-feature=exemplar-storage to the Prometheus process invocation (the command list where '--config.file=...', '--storage.tsdb.path=...', etc. are defined) so that exemplars will be persisted and Grafana's exemplarTraceIdDestinations (in your grafana-datasources.yaml) can link metrics to Tempo traces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@telemetry-demo/grafana-datasources.yaml`:
- Around line 1-13: The Grafana datasource config enables
exemplarTraceIdDestinations for Prometheus but Prometheus must be started with
exemplar storage enabled; update the Prometheus service startup (the Prometheus
container/service that runs Prometheus) to include the flag
--enable-feature=exemplar-storage so exemplars are persisted and
exemplarTraceIdDestinations in Prometheus (the Prometheus datasource) will
produce data.
In `@telemetry-demo/logs/scanoss/api/.gititnore`:
- Around line 1-2: Rename the misnamed ignore file `.gititnore` to `.gitignore`
so Git applies the `*.log` rule; locate the file named `.gititnore` in the
project and move/rename it to `.gitignore` (preserve the file contents exactly),
then verify that `*.log` is now respected by Git.
In `@telemetry-demo/README.md`:
- Line 67: The numbered list item in README.md contains a missing space after
the numeral; update the list line that currently reads "3.Type `scanoss` into
the \"Search metric\" field to list all SCANOSS metrics" to include a space
after the period so it becomes "3. Type `scanoss` into the \"Search metric\"
field to list all SCANOSS metrics".
- Line 144: Fix the README table row that currently reads "`tempo.yml` | Defines
Temop traces setup" by correcting the typo "Temop" to "Tempo" and updating the
filename token from `tempo.yml` to `tempo.yaml` so the table entry reads
"`tempo.yaml` | Defines Tempo traces setup"; ensure the corrected text appears
in the README table cell that contains the original "`tempo.yml`" entry.
- Line 41: Fix the typo in the README sentence that currently reads "To logins
to Grafana, please browse the following URL:" by changing it to "To login to
Grafana, please browse the following URL:"; locate the exact line in
telemetry-demo/README.md containing that sentence and update the wording
accordingly.
- Around line 34-37: Fix the Markdown fence in the README example: the one-off
scan code block starts with three backticks but ends with four; update the
closing fence to three backticks so the block is properly fenced (the example
showing the curl command / echo 'file=056f9b... | curl -s -X POST ... -F
'file=@-;filename=test.wfp'' should be wrapped with matching ``` on both start
and end).
In `@telemetry-demo/tempo.yaml`:
- Around line 29-37: The Tempo config defines metrics_generator.remote_write to
push to Prometheus but Prometheus is missing the required remote-write receiver
flag; update the Prometheus service command in docker-compose.yml to include the
--web.enable-remote-write-receiver flag (alongside the existing --config.file
entry) so that Tempo's metrics_generator.remote_write endpoint is accepted and
remote writes are not rejected.
---
Outside diff comments:
In `@telemetry-demo/docker-compose.yml`:
- Around line 61-67: The Prometheus container command block is missing the
feature flag to store exemplars; add the flag --enable-feature=exemplar-storage
to the Prometheus process invocation (the command list where
'--config.file=...', '--storage.tsdb.path=...', etc. are defined) so that
exemplars will be persisted and Grafana's exemplarTraceIdDestinations (in your
grafana-datasources.yaml) can link metrics to Tempo traces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb29cd7d-2d89-49f5-8cc8-0bf8a5388a6c
📒 Files selected for processing (10)
telemetry-demo/README.mdtelemetry-demo/TELEMETRY_CONFIG.mdtelemetry-demo/config/app-config-demo.jsontelemetry-demo/docker-compose.ymltelemetry-demo/grafana-datasources.yamltelemetry-demo/logs/scanoss/api/.gititnoretelemetry-demo/loki.yamltelemetry-demo/otel-collector-config.ymltelemetry-demo/promtail-config.ymltelemetry-demo/tempo.yaml
telemetry-demo/README.md
Outdated
| | `promtail-config.yml` | Configures Promtail log Collector pipeline | | ||
| | `loki.yaml` | Defines Loki logging setup | | ||
| | `prometheus.yml` | Defines Prometheus scrape targets | | ||
| | `tempo.yml` | Defines Temop traces setup | |
There was a problem hiding this comment.
Typo: "Temop" should be "Tempo".
📝 Proposed fix
-| `tempo.yml` | Defines Temop traces setup |
+| `tempo.yaml` | Defines Tempo traces setup |Note: The filename in the table should also be tempo.yaml (not tempo.yml) to match the actual filename added in this PR.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `tempo.yml` | Defines Temop traces setup | | |
| | `tempo.yaml` | Defines Tempo traces setup | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@telemetry-demo/README.md` at line 144, Fix the README table row that
currently reads "`tempo.yml` | Defines Temop traces setup" by correcting the
typo "Temop" to "Tempo" and updating the filename token from `tempo.yml` to
`tempo.yaml` so the table entry reads "`tempo.yaml` | Defines Tempo traces
setup"; ensure the corrected text appears in the README table cell that contains
the original "`tempo.yml`" entry.
dopicatto
approved these changes
Mar 20, 2026
dopicatto
left a comment
dopicatto
left a comment
There was a problem hiding this comment.
Looks good overall — nice addition to the observability demo. A couple of functional things to address before merging:
Prometheus flags (both Major):
The metrics_generator in tempo.yaml does remote write to Prometheus, and grafana-datasources.yaml has exemplarTraceIdDestinations configured — but the Prometheus service in docker-compose.yml is missing both required flags:
yamlcommand:
- '--config.file=/etc/prometheus/prometheus.yml'
- '--web.enable-remote-write-receiver'
- '--enable-feature=exemplar-storage'
Without these, Tempo's remote write will silently 404 and exemplar trace linking won't produce any data.
Minor typos (easy fixes):
logs/scanoss/api/.gititnore → rename to .gitignore
README: closing fence has 4 backticks instead of 3
README: "To logins to Grafana" → "To login to"
README: "Temop" → "Tempo", and tempo.yml → tempo.yaml to match actual filename
README: missing space after "3." in numbered list
There was a problem hiding this comment.
🧹 Nitpick comments (3)
telemetry-demo/docker-compose.yml (3)
108-118: Remove commented-out code.The commented-out
GF_SECURITY_ADMIN_PASSWORDblock (lines 111-112) is unnecessary clutter. Since anonymous admin access is now used for dev convenience, the old configuration can be deleted.🧹 Proposed cleanup
environment: - GF_AUTH_ANONYMOUS_ENABLED=true - GF_AUTH_ANONYMOUS_ORG_ROLE=Admin # dev convenience — lock down for prod -# environment: -# - GF_SECURITY_ADMIN_PASSWORD=admin volumes: - grafana_data:/var/lib/grafana🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telemetry-demo/docker-compose.yml` around lines 108 - 118, Remove the commented-out GF_SECURITY_ADMIN_PASSWORD environment block to clean up unused configuration: delete the two comment lines containing "# environment:" and "# - GF_SECURITY_ADMIN_PASSWORD=admin" (the commented GF_SECURITY_ADMIN_PASSWORD entry) so only the active environment variables (GF_AUTH_ANONYMOUS_ENABLED and GF_AUTH_ANONYMOUS_ORG_ROLE) remain.
128-133: Minor: inconsistent volume naming convention.
promtail-positionsuses hyphens while other volumes use underscores (prometheus_data,grafana_data,loki_data,tempo_data). Consider usingpromtail_positionsfor consistency.🧹 Proposed fix
volumes: prometheus_data: grafana_data: loki_data: tempo_data: - promtail-positions: + promtail_positions:Also update the reference in the promtail service:
volumes: - ./promtail-config.yml:/etc/promtail/promtail-config.yml:ro - ./logs/scanoss/api:/var/log/scanoss/api:ro - - promtail-positions:/tmp + - promtail_positions:/tmp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telemetry-demo/docker-compose.yml` around lines 128 - 133, Rename the inconsistent volume name promtail-positions to promtail_positions to match the underscore convention used by prometheus_data, grafana_data, loki_data, and tempo_data; then update the corresponding volume reference in the promtail service definition (the promtail service's volumes entry that references promtail-positions) so it uses promtail_positions instead.
32-37: Consider depending ontempoinstead ofloki.The otel-collector exports traces to Tempo (per the config), so it should depend on
temporather thanloki. Loki receives logs from Promtail, not from the otel-collector.♻️ Proposed fix
depends_on: - prometheus - - loki + - tempo networks: - monitoring restart: unless-stopped🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telemetry-demo/docker-compose.yml` around lines 32 - 37, The service's depends_on currently lists 'loki' but the otel-collector exports traces to Tempo; update the depends_on block to depend on 'tempo' instead of 'loki' (replace the 'loki' entry with 'tempo' in the depends_on array) and verify there is a 'tempo' service defined in the compose file so the dependency is valid (adjust or add the 'tempo' service if missing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@telemetry-demo/docker-compose.yml`:
- Around line 108-118: Remove the commented-out GF_SECURITY_ADMIN_PASSWORD
environment block to clean up unused configuration: delete the two comment lines
containing "# environment:" and "# - GF_SECURITY_ADMIN_PASSWORD=admin"
(the commented GF_SECURITY_ADMIN_PASSWORD entry) so only the active environment
variables (GF_AUTH_ANONYMOUS_ENABLED and GF_AUTH_ANONYMOUS_ORG_ROLE) remain.
- Around line 128-133: Rename the inconsistent volume name promtail-positions to
promtail_positions to match the underscore convention used by prometheus_data,
grafana_data, loki_data, and tempo_data; then update the corresponding volume
reference in the promtail service definition (the promtail service's volumes
entry that references promtail-positions) so it uses promtail_positions instead.
- Around line 32-37: The service's depends_on currently lists 'loki' but the
otel-collector exports traces to Tempo; update the depends_on block to depend on
'tempo' instead of 'loki' (replace the 'loki' entry with 'tempo' in the
depends_on array) and verify there is a 'tempo' service defined in the compose
file so the dependency is valid (adjust or add the 'tempo' service if missing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84ce180a-f7a5-4361-a86d-6646fe9268d4
📒 Files selected for processing (6)
.github/workflows/go-ci.yml.github/workflows/golangci-lint.yml.github/workflows/release.ymltelemetry-demo/README.mdtelemetry-demo/docker-compose.ymltelemetry-demo/logs/scanoss/api/.gitignore
✅ Files skipped from review due to trivial changes (3)
- telemetry-demo/logs/scanoss/api/.gitignore
- .github/workflows/golangci-lint.yml
- .github/workflows/go-ci.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- telemetry-demo/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores