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

Handle "mixed mode" OT/OTel tracing in OT Bridge #161

Closed
johananl opened this issue Sep 23, 2019 · 4 comments · Fixed by #924
Closed

Handle "mixed mode" OT/OTel tracing in OT Bridge #161

johananl opened this issue Sep 23, 2019 · 4 comments · Fixed by #924
Assignees
Labels
release:required-for-ga To be resolved before GA release shim OpenTracing or OpenCensus compatibility tracing

Comments

@johananl
Copy link
Contributor

@krnowak brought to my attention the following use case for the OT bridge:

  • Library A is instrumented with OT and uses the OT bridge. Library A creates a parent span.
  • Library B is instrumented with OTel directly. Library B creates a child span whose parent is the span library A created.

Do we want the OT bridge to support this use case by handling parent-child relationship between the wrapped OT span and the OTel span?

@Oberon00
Copy link
Member

I think this would work out of the box with the straightforward implementation of basically delegating the OT functions to OTel mostly one to one. Or what do you think would break that interaction?

@krnowak
Copy link
Member

krnowak commented Sep 23, 2019

I don't know how storing of the active span is done in python, but in Go we use the context.Context to store the current span. So for the scenario described above, the OT bridge would take care of putting the OT span into the context to mark it as an active OT span, while the Otel tracer that gets the call from the bridge would setup the context with the Otel span as a current Otel span.

Other scenario was more "interesting" - library A uses Otel and then it calls library B that uses OT. So at this point it would be nice for the library B if the Otel span created by library A was also wrapped and put into the context as an active OT span. That way, an OT span created inside the library B would have a proper parent - a OT span that wraps an Otel span created in library A. The "interesting" part here is that library A uses Otel API, so no OT API is used directly and yet some OT stuff needs to happen too.

@Oberon00
Copy link
Member

Oberon00 commented Sep 23, 2019

Looking at the OpenTelemetry code, I would implement a scope manager that sets / gets the active Span from the OpenTelemetry tracers (of course, setting is not possible ATM, #154 may help but you may need to add a new setter to the OTel Tracer).

EDIT: Regarding the type mismatch: Either you always return a new OTAdaptedOTelSpan(otel_span) or, if you think that might break someone, just monkeypatch a _opentracing_span property into the OTel span and reuse that.

@c24t
Copy link
Member

c24t commented Dec 5, 2019

Removing this from milestones for now, @carlosalberto to consider adding for v4.

@c24t c24t removed this from the Alpha v0.3 milestone Dec 5, 2019
@ocelotl ocelotl self-assigned this Jul 11, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 18, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 18, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 20, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 20, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 22, 2020
@codeboten codeboten added the release:required-for-ga To be resolved before GA release label Jul 22, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 31, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 5, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 6, 2020
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 6, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
* feat(plugin): add http plugin

Closes open-telemetry#157

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: integrate vmarchaud recommandations

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: wip revert - gts fix issue on all packages

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: integrate mayurkale22 recommendations

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* ci: gts fix for ci build

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* refactor: add mayurkale22 recommendations

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: integrate bg451 recommendations

test: increase coverage
fix: attributes requirements from the spec.

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add missing header and revert scopeManager to private field

test: rename some tests
fix: copy/paste tests

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add tests and improve attributes from spec

fix parentSpanId instead of parentId from rebase
add workaround with got and node12+ (real http call)
improve args passed to function (url, options, cb)

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add mayurkale22 recommendations

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: rebase and remove/replace wrapEmitter to bind

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add Flarna recommendations

fix: add Flarna recommendations
fix: tests
fix: OC bugs in OT only
test: add assertions
Allow options object as second argument

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* refactor: simplify propagation usage

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add Flarna recommandations

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* refactor(test): use ReadableSpan

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* feat: export class/enums for https module

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* refactor: plugin.enable has logger param

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add license header, rename enum files and refactoring

refactor: make integration tests mandatory for ci only
remove duplicate tests
remove dead comments

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:required-for-ga To be resolved before GA release shim OpenTracing or OpenCensus compatibility tracing
Projects
None yet
6 participants