-
Notifications
You must be signed in to change notification settings - Fork 459
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
tracer: merge support for OTel API #1839
Conversation
Added an OTel API skeleton & span methods
* added an otel api skeleton & some span methods * lint & copyright * fixed lint * updated comments * updated span operations * updated tracer / tracerProvider * fixed lint & copyright * fixed lint * updated according to comments * updated tests + add simple benchmark * fixed lint & copyright * removed a comment * Add BenchmarkOtelConcurrentTracing * added span testing function stubs * added WIP unit tests for TestForceFlush, TestShutdown * fixed TestForceshutdown log error * refactored span method testing, refined otel tracer test fns * added TestForceFlush * commented use of waitForTestAgent * added env var testing * remove test case * added unit test for SpanContext * Update ddtrace/opentelemetry/span_test.go Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> * Update ddtrace/opentelemetry/span_test.go Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> * refactored test cases * removed new lines * fixed blocking test cases * updated test agent * Update ddtrace/opentelemetry/span_test.go Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> * Update ddtrace/opentelemetry/span_test.go Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> * unmarshal payload * refactor test tracer provider * fix lint: move ctx.Context to first argument for waitForPayload * lint * unit test for test tracer start options * fix flaky TestSpanWithContext * fix flaky agent timeout * refactored waitForPayload * flip success bool forceflush * update go.mod * Update ddtrace/opentelemetry/tracer_test.go Co-authored-by: Katie Hockman <katie@hockman.dev> * Update ddtrace/opentelemetry/span_test.go Co-authored-by: Katie Hockman <katie@hockman.dev> * cleanup/refactor testing code according to comments * use iota for flushstatus constants * refactor span set status test * fix typos * put test case for span end inline * naming consistency for span end test * prevent flaky forceflush test * use null logger * remove null logger --------- Co-authored-by: Diana Shevchenko <diana.shevchenko@datadoghq.com> Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> Co-authored-by: Katie Hockman <katie@hockman.dev>
BenchmarksComparing candidate commit fb0c2dd in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 17 metrics, 0 unstable metrics. scenario:BenchmarkConcurrentTracing-24
|
We should revert #1849 as part of this PR. |
We have run locally the parametric tests with all the tests unskipped |
But we still want to tests to run on main as well right? |
Sure, but for that we have to remove the fix and unskip the tests in parametric, introducing a blocker again. So for now, we're running tests manually |
Unskipping the tests won't break main since we're pinning an older version of system-tests that doesn't contain otel tests at all |
This comment is not related to a previous comment of Ahmed. |
Sorry for the confusion lol! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Adds support for an OpenTelemetry compatible API which wraps the Datadog API.
What does this PR do?
This PR adds support for an OpenTelemetry compatible API which wraps the Datadog API.
Motivation
This can make it easier for dd-trace-go users who have already instrumented their code with OpenTelemetry and want to utilize the features of the dd-trace-go library with as few code changes as possible. Migrating from using the upstream OTel SDK (or another OTel API implementation) to using this Datadog OTel API should be as simple as subbing out their existing
TracerProvider
for ours.Describe how to test/QA your changes
Reviewer's Checklist
Triage
milestone is set.