Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove OpenTelemetry extensions #1917

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Remove OpenTelemetry extensions #1917

merged 1 commit into from
Feb 24, 2022

Conversation

delner
Copy link
Contributor

@delner delner commented Feb 24, 2022

ddtrace contains some dead code that amends SpanOperation with OpenTelemetry behavior. OpenTelemetry shouldn't use Datadog::Tracing directly, instead it should use OTLP, so this code is never called.

@delner delner added feature Involves a product feature deprecation Involves a deprecation labels Feb 24, 2022
@delner delner added this to the 1.0.0.beta2 milestone Feb 24, 2022
@delner delner self-assigned this Feb 24, 2022
@delner delner requested a review from a team February 24, 2022 16:47
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love PRs that just delete stuff. The change seems reasonable, so I've approved it.

I noticed we still have two files referencing opentelemetry:

./docs/UpgradeGuide.md:| `ddtrace/opentelemetry`     | `datadog/opentelemetry`     |
./Rakefile:    t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,opentracer,opentelemetry,auto_instrument}/**/*_spec.rb,'\
./Rakefile:  RSpec::Core::RakeTask.new(:opentelemetry) do |t, args|
./Rakefile:    t.pattern = 'spec/datadog/opentelemetry/**/*_spec.rb'
./Rakefile:  declare '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ jruby' => 'bundle exec rake spec:opentelemetry'

Finally -- I don't quite understand the description of the PR. Could you clarify what "using OTLP" is and how it looks like?

(And should we include guidelines for using OTLP in the 1.0 upgrade notes?)

@delner
Copy link
Contributor Author

delner commented Feb 24, 2022

Removing other references was a good catch. Updated those.

In short, OTLP is a way for OpenTracing to send its traces directly to the Datadog agent, bypassing our tracing library entirely. This is the preferred way to utilize OpenTracing with Datadog. It's described here: https://docs.datadoghq.com/tracing/setup_overview/open_standards/#otlp-ingest-in-datadog-agent We can add documentation for this later.

@delner delner merged commit 72dd9dc into master Feb 24, 2022
@delner delner deleted the remove_otel_extensions branch February 24, 2022 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Involves a deprecation feature Involves a product feature
Projects
1.0
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

2 participants