Conversation
| class Logger | ||
| class << self | ||
| def increment(name, tags: {}, by: 1) | ||
| if OpenTelemetryExporter.configuration.opentelemetry_enabled? |
There was a problem hiding this comment.
Maybe you could have a class method OpenTelemetryExporter.enabled?
| if OpenTelemetryExporter.configuration.opentelemetry_enabled? | ||
| begin | ||
| meter = OpenTelemetryExporter.configuration.opentelemetry_meter | ||
| raise "OpenTelemetry meter not configured" unless meter | ||
|
|
||
| meter.create_counter(name).add(by, attributes: convert_tags(tags)) | ||
| by | ||
| rescue => e | ||
| log_error("OpenTelemetry error for metric '#{name}': #{e.message}") | ||
| log_error("Falling back to configured fallback") | ||
| call_fallback_increment(name, tags: tags, by: by) | ||
| end | ||
| else | ||
| call_fallback_increment(name, tags: tags, by: by) | ||
| end |
There was a problem hiding this comment.
And perhaps better as guard clause?
| if OpenTelemetryExporter.configuration.opentelemetry_enabled? | |
| begin | |
| meter = OpenTelemetryExporter.configuration.opentelemetry_meter | |
| raise "OpenTelemetry meter not configured" unless meter | |
| meter.create_counter(name).add(by, attributes: convert_tags(tags)) | |
| by | |
| rescue => e | |
| log_error("OpenTelemetry error for metric '#{name}': #{e.message}") | |
| log_error("Falling back to configured fallback") | |
| call_fallback_increment(name, tags: tags, by: by) | |
| end | |
| else | |
| call_fallback_increment(name, tags: tags, by: by) | |
| end | |
| call_fallback_increment(name, tags: tags, by: by) unless OpenTelemetryExporter.enabled? | |
| meter = OpenTelemetryExporter.configuration.opentelemetry_meter | |
| raise "OpenTelemetry meter not configured" unless meter | |
| meter.create_counter(name).add(by, attributes: convert_tags(tags)) | |
| by | |
| rescue => e | |
| log_error("OpenTelemetry error for metric '#{name}': #{e.message}") | |
| log_error("Falling back to configured fallback") | |
| call_fallback_increment(name, tags: tags, by: by) |
| return by | ||
| end | ||
|
|
||
| if defined?(Datadog::Statsd) && fallback_object.is_a?(Datadog::Statsd) |
There was a problem hiding this comment.
There's a lot of duplication, and if this is meant to be "extensible" (to other libraries), it shouldn't be hard-coded to Datadog::Statsd. Feels like these belong in some OpenTelemetryExporter::DataDogAdapter or something like that, with some sort of factory method returning whatever the configured (or default) fallback adapter is.
There was a problem hiding this comment.
As a matter of fact, maybe even OpenTelemetry itself should be an adapter, and then this gem is a more "generic", non-OpenTelemetry specific gem (maybe just TelemetryExporter?) and you can specify OpenTelemetry as your adapter of choice, or Datadog). Similar to how ActiveJob abstracts away whichever background job system you want to use
| tags.each_with_object({}) do |tag, result| | ||
| key, value = tag.split(":", 2) | ||
| result[key] = value | ||
| end |
There was a problem hiding this comment.
| tags.each_with_object({}) do |tag, result| | |
| key, value = tag.split(":", 2) | |
| result[key] = value | |
| end | |
| tags.map { |tag| tag.split(":", 2) }.to_h |
2 participants
No description provided.