Conversation
joshwestbrook
force-pushed
the
open-telemetry
branch
2 times, most recently
from
a9bb678 to
7008f29
Compare
joshwestbrook
force-pushed
the
open-telemetry
branch
from
7008f29 to
201635d
Compare
driv3r
left a comment
driv3r
left a comment
There was a problem hiding this comment.
It can be simplified quite a bit regarding optional gems, as DD statsd was optional from the start anyway, plus you have to initialize the client manually - so there's no automatic setup for this, so we don't actually reference any 3rd party classes.
Gemfile
Outdated
| gem 'opentelemetry-metrics-api', github: 'open-telemetry/opentelemetry-ruby', glob: 'metrics_api/*.gemspec' # TODO: Once gauges are released, we can switch back to released version: https://github.com/open-telemetry/opentelemetry-ruby/commit/bb5159598850b42e9da54608a8af2fbe422193b7 | ||
| gem 'opentelemetry-metrics-sdk', github: 'open-telemetry/opentelemetry-ruby', glob: 'metrics_sdk/*.gemspec' # TODO: Once gauges are released, we can switch back to released version: https://github.com/open-telemetry/opentelemetry-ruby/commit/bb5159598850b42e9da54608a8af2fbe422193b7 | ||
| gem 'opentelemetry-sdk' | ||
| end |
There was a problem hiding this comment.
groups don't really matter in gems Gemfile, as it's used exclusively in development, only gems from .gemspec matter for end users
README.md
Outdated
|
|
||
| Given gem provides built in target for OpenTelemetry Metrics SDK, that uses batch operations to publish metrics. | ||
|
|
||
| **NOTE** Be sure to have `opentelemetry-metrics-sdk` gem installed. |
There was a problem hiding this comment.
I would maybe update this not to include the gemfile snippet, i.e.
gem "opentelemetry-metrics-sdk"and update the same note for dogstatsd
| require 'datadog/statsd' | ||
| rescue LoadError | ||
| # Gracefully handle the case when Datadog::Statsd is not installed | ||
| end |
There was a problem hiding this comment.
there's no need for this, as:
- it worked so far without explicit require already as it was optional from the start :)
- it would crash on the configuration with explicit message as well, as it's required to provide initialized client, like
config.add_target :dogstatsd, client: Datadog::Statsd.newsame for the open telemetry
|
@joshwestbrook also sorry for such a long reply! Just came back from holidays |
No worries at all! Thanks for taking a look |
|
As this repo seems… abandoned? I've forked and started adding things we need in production, including Puma 7 support. I'm currently adding OTel support too. |
3 participants
Add support for an Open Telemetry target as an alternative to DataDog.
Wasn't sure of the best way to make it so that consumers of this gem don't have to pull in both the
dogstatsd-rubyandopentelemetry-*gems in order to use one of the targets. Open to any suggestions! 😄