Skip to content

Commit 63aefde

Browse files
committed
fix(examples): address dice roller PR review feedback
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 #2048 Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
1 parent 69adddc commit 63aefde

4 files changed

Lines changed: 28 additions & 40 deletions

File tree

examples/dice_roller/README.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ OTEL_LOGS_EXPORTER=otlp \
4848
OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 \
4949
OTEL_SERVICE_NAME=dice_roller \
5050
ruby app.rb
51+
You can also export your telemetry to more than one location. To send to an OpenTelemetry Collector and to the console, set the exporter env vars:
52+
53+
```bash
54+
OTEL_TRACES_EXPORTER=otlp,console \
55+
OTEL_METRICS_EXPORTER=otlp,console \
56+
OTEL_LOGS_EXPORTER=otlp,console \
57+
OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 \
58+
OTEL_SERVICE_NAME=dice_roller \
59+
ruby app.rb
5160
```
5261

5362
---
@@ -177,4 +186,4 @@ All Ruby `Logger` output is bridged to OpenTelemetry via
177186
| `OTEL_METRICS_EXPORTER` | `console` | Metrics exporter (`console` or `otlp`) |
178187
| `OTEL_LOGS_EXPORTER` | `console` | Logs exporter (`console` or `otlp`) |
179188
| `OTEL_EXPORTER_OTLP_ENDPOINT` | `http://localhost:4318` | OTLP endpoint |
180-
| `OTEL_LOG_LEVEL` | _(unset)_ | OTel internal diagnostic log level (e.g. `debug`) |
189+
| `OTEL_LOG_LEVEL` | _(unset)_ | OTel internal log level (e.g. `debug`, default: `info`) |

examples/dice_roller/instrumented/app.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,15 @@ class DiceApp < Sinatra::Base
3838
rolls_param = params[:rolls]
3939
player = params[:player]
4040

41-
# Default rolls to 1 if not provided
42-
rolls = rolls_param.nil? ? 1 : rolls_param.to_i
43-
4441
# Validate: must be a number
4542
unless rolls_param.nil? || rolls_param.match?(/\A-?\d+\z/)
4643
status 400
4744
LOGGER.warn "Invalid rolls parameter: #{rolls_param}"
48-
return { status: 'error', message: 'Parameter rolls must be a positive integer' }.to_json
45+
return { status: 'error', message: 'Parameter rolls must be an integer' }.to_json
4946
end
5047

48+
rolls = rolls_param.nil? ? 1 : rolls_param.to_i
49+
5150
dice = Dice.new
5251

5352
begin

examples/dice_roller/instrumented/otel.rb

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -48,38 +48,19 @@
4848
end
4949

5050
# ---------------------------------------------------------------------------
51-
# Metrics SDK — add a console reader so metrics are visible without a backend
51+
# Metrics and Logs Configuration
5252
# ---------------------------------------------------------------------------
53-
case ENV['OTEL_METRICS_EXPORTER']
54-
when 'otlp'
55-
otlp_metric_exporter = OpenTelemetry::Exporter::OTLP::Metrics::MetricsExporter.new
56-
OpenTelemetry.meter_provider.add_metric_reader(
57-
OpenTelemetry::SDK::Metrics::Export::PeriodicMetricReader.new(exporter: otlp_metric_exporter)
58-
)
59-
else
60-
console_metric_exporter = OpenTelemetry::SDK::Metrics::Export::ConsoleMetricPullExporter.new
61-
OpenTelemetry.meter_provider.add_metric_reader(console_metric_exporter)
62-
end
63-
64-
# ---------------------------------------------------------------------------
65-
# Logs SDK — wire up a console exporter and bridge Ruby's Logger
66-
# ---------------------------------------------------------------------------
67-
logs_logger_provider = OpenTelemetry::SDK::Logs::LoggerProvider.new
68-
case ENV['OTEL_LOGS_EXPORTER']
69-
when 'otlp'
70-
log_processor = OpenTelemetry::SDK::Logs::Export::BatchLogRecordProcessor.new(
71-
OpenTelemetry::Exporter::OTLP::Logs::LogsExporter.new
72-
)
73-
logs_logger_provider.add_log_record_processor(log_processor)
74-
else
75-
log_processor = OpenTelemetry::SDK::Logs::Export::SimpleLogRecordProcessor.new(
76-
OpenTelemetry::SDK::Logs::Export::ConsoleLogRecordExporter.new
77-
)
78-
logs_logger_provider.add_log_record_processor(log_processor)
79-
end
80-
OpenTelemetry.logger_provider = logs_logger_provider
81-
53+
# NOTE: Metrics and logs providers are automatically configured by
54+
# OpenTelemetry::SDK.configure above based on the OTEL_METRICS_EXPORTER
55+
# and OTEL_LOGS_EXPORTER environment variables. The SDK handles:
56+
#
57+
# - Creating the meter_provider and logger_provider
58+
# - Setting up appropriate exporters (console, otlp, etc.)
59+
# - Adding metric readers and log record processors
60+
#
61+
# However, we still need to explicitly shutdown the providers at exit
62+
# to ensure all telemetry data is flushed before the application terminates.
8263
at_exit do
8364
OpenTelemetry.meter_provider.shutdown
84-
logs_logger_provider.shutdown
65+
OpenTelemetry.logger_provider.shutdown
8566
end

examples/dice_roller/uninstrumented/app.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,15 @@ class DiceApp < Sinatra::Base
2929
rolls_param = params[:rolls]
3030
player = params[:player]
3131

32-
# Default rolls to 1 if not provided
33-
rolls = rolls_param.nil? ? 1 : rolls_param.to_i
34-
3532
# Validate: must be a number
3633
unless rolls_param.nil? || rolls_param.match?(/\A-?\d+\z/)
3734
status 400
3835
LOGGER.warn "Invalid rolls parameter: #{rolls_param}"
39-
return { status: 'error', message: 'Parameter rolls must be a positive integer' }.to_json
36+
return { status: 'error', message: 'Parameter rolls must be an integer' }.to_json
4037
end
4138

39+
rolls = rolls_param.nil? ? 1 : rolls_param.to_i
40+
4241
dice = Dice.new
4342

4443
begin

0 commit comments

Comments
 (0)