Skip to content

Remove legacy hook for OpenTracing#899

Merged
SammyK merged 1 commit intomasterfrom
sammyk/build/open-tracing
May 22, 2020
Merged

Remove legacy hook for OpenTracing#899
SammyK merged 1 commit intomasterfrom
sammyk/build/open-tracing

Conversation

@SammyK
Copy link
Copy Markdown
Contributor

@SammyK SammyK commented May 22, 2020 •
edited
Loading

Description

This PR removes the legacy API from the OpenTracing implementation. This introduces a breaking change in the way OpenTracing is initialized with ddtrace. Before this change, an OpenTracing-compatible tracer (\DDTrace\OpenTracer\Tracer) would be automatically returned from \OpenTracing\GlobalTracer::get(). After this change, the OpenTracing-compatible tracer must be set manually.

# Before
$otTracer = \OpenTracing\GlobalTracer::get();

# After
$otTracer = new \DDTrace\OpenTracer\Tracer(\DDTrace\GlobalTracer::get());
\OpenTracing\GlobalTracer::set($otTracer);

The OpenTracing documentation will be updated as part of this PR.

Edit: Preview link of the documentation changes from DataDog/documentation#7528.

Readiness checklist

  • (only for Members) Changelog has been added to the appropriate release draft. Create one if necessary.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the appropriate release draft. For community contributors the reviewer is in charge of this task.

@SammyK SammyK added 💔 breaking-change 🍏 core Changes to the core tracing functionality labels May 22, 2020
@SammyK SammyK added this to the 0.46.0 milestone May 22, 2020
@SammyK SammyK mentioned this pull request May 22, 2020
Merged
@SammyK SammyK merged commit 65ea163 into master May 22, 2020
@SammyK SammyK deleted the sammyk/build/open-tracing branch May 22, 2020 16:32
@grantlucas
Copy link
Copy Markdown

grantlucas commented Jun 3, 2020

Sorry if this isn't the correct place for questions, but the release notes linked here first so I figured I'd give it a shot. I'm not 100% clear if I need to update our application to support 0.46.0. We're not using opentracing/opentracing but datadog/dd-trace instead, and our typical usage is, for example, getting the root span with

$rootSpan = \DDTrace\GlobalTracer::get()
    ->getRootScope()
    ->getSpan();

or starting a new span with

/* @var \DDTrace\Scope $scope */
$scope = \DDTrace\GlobalTracer::get()->startActiveSpan($spanName);

/* @var \DDTrace\Span $span */
$span  = $scope->getSpan();

If that's our main usage, and we're not using opentracing/opentracing package, are updates needed for our codebase to support 0.46.0?

Thank you for the help and the great package!

@labbati
Copy link
Copy Markdown
Member

labbati commented Jun 3, 2020

... we're not using opentracing/opentracing package

No, nothing specific needs to be done for the current PR.

We're not using opentracing/opentracing but datadog/dd-trace instead

Starting from 0.46.0 you MUST register datadog/dd-trace as a dependency to your docker compose file. Most probably you were already doing and in that case nothing else is required.

@grantlucas
Copy link
Copy Markdown

grantlucas commented Jun 3, 2020

Great, thanks for the confirmation!

@labbati labbati mentioned this pull request Jun 12, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morrisonlevi morrisonlevi morrisonlevi approved these changes

Assignees

No one assigned

Labels

💔 breaking-change 🍏 core Changes to the core tracing functionality

Projects

None yet

Milestone

0.46.0

Development

Successfully merging this pull request may close these issues.

4 participants

@SammyK @grantlucas @labbati @morrisonlevi