feat(examples): add dice roller getting started reference application#2048
Conversation
bc542a7 to
c8cd36d
Compare
Add a dice roller example application as the reference implementation for the OpenTelemetry Getting Started guide, as specified in: https://opentelemetry.io/docs/getting-started/reference-application-specification/ Includes: - Uninstrumented version (baseline Sinatra app) - Instrumented version with OpenTelemetry SDK, traces, metrics and logs - Dockerfile for containerized usage - README with setup and usage instructions - GitHub Actions workflow and composite action for CI Closes open-telemetry#1928 Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
c8cd36d to
2fa5c93
Compare
|
Thank you for opening this, @arjun-rajappa! Taking a look today. |
|
Hi @arjun-rajappa - In the middle of a review, but not quite done yet. Will pick things up tomorrow. Sorry about the delay here. |
kaylareopelle
left a comment
There was a problem hiding this comment.
This is awesome, @arjun-rajappa! Thank you so much for this contribution. I think we're really close to being ready to merge. The main issue I see is one that @hannahramadan pointed out in her review.
The spec says:
if rolls is set to a 0 or a negative integer: status code 500 and no JSON output. The error examples will be used to demonstrate how OpenTelemetry can be used to identify errors.
There's a bit of cleanup to do to incorporate this behavior. Check out @hannahramadan's comments for good callouts. That was the main issue I noticed. I'll re-review after the 500 feature is added to the app.
Add container resource detector based on review feedback. Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
kaylareopelle
left a comment
There was a problem hiding this comment.
Thank you for the updates, @arjun-rajappa! I noticed a few more things.
Remove redundant metrics and logs configuration code that is already handled by OpenTelemetry::SDK.configure through configuration hooks. Improve validation and code organization based on review comments. Changes: - Remove manual metrics/logs provider setup (handled by SDK configurator) - Use OpenTelemetry.logger_provider.shutdown for consistency - Move rolls assignment after validation for better code flow - Update regex to only accept positive integers (/\A\d+\z/) - Add multi-exporter example to README - Document OTEL_LOG_LEVEL default value - Fix markdown linting errors Addresses review feedback from PR open-telemetry#2048 Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
Remove redundant metrics and logs configuration code that is already handled by OpenTelemetry::SDK.configure through configuration hooks. Improve validation and code organization based on review comments. Changes: - Remove manual metrics/logs provider setup (handled by SDK configurator) - Use OpenTelemetry.logger_provider.shutdown for consistency - Move rolls assignment after validation for better code flow - Update regex to only accept positive integers (/\A\d+\z/) - Add multi-exporter example to README - Document OTEL_LOG_LEVEL default value - Fix markdown linting errors Addresses review feedback from PR open-telemetry#2048 Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
eef0f5c to
c2bcdb7
Compare
Remove redundant metrics and logs configuration code that is already handled by OpenTelemetry::SDK.configure through configuration hooks. Improve validation and code organization based on review comments. Changes: - Remove manual metrics/logs provider setup (handled by SDK configurator) - Use OpenTelemetry.logger_provider.shutdown for consistency - Move rolls assignment after validation for better code flow - Add multi-exporter example to README - Document OTEL_LOG_LEVEL default value - Fix markdown linting errors Addresses review feedback from PR open-telemetry#2048 Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
c2bcdb7 to
63aefde
Compare
xuan-cao-swi
left a comment
There was a problem hiding this comment.
LGTM, please fix the Spelling Check ci
|
|
||
| | Span name | Kind | Attributes | | ||
| |-----------------|------------|---------------------------------------------------------------------| | ||
| | `GET /rolldice` | `SERVER` | HTTP semantic conventions (via Sinatra instrumentation) | |
There was a problem hiding this comment.
As this is an endpoint mentioned in the specification - incontent/en/docs/getting-started/reference-application-specification.md
Will be adding this word to ignored words CI / Spelling Check
The 'rolldice' endpoint is mentioned in the specification at content/en/docs/getting-started/reference-application-specification.md and should be excluded from spell checking. Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
kaylareopelle
left a comment
There was a problem hiding this comment.
Thank you for all your adjustments @arjun-rajappa! I noticed one small markdown formatting thing. After that's addressed, this is good to go.
| OTEL_LOGS_EXPORTER=otlp \ | ||
| OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 \ | ||
| OTEL_SERVICE_NAME=dice_roller \ | ||
| ruby app.rb |
There was a problem hiding this comment.
Small formatting suggestion:
| ruby app.rb | |
| ruby app.rb | |
| ``` | |
Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
Add a dice roller example application as the reference implementation for the OpenTelemetry Getting Started guide, as specified in: https://opentelemetry.io/docs/getting-started/reference-application-specification/
Includes:
Closes #1928