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

Add support for jaeger propagation formats #696

Closed
vladislav-kiva opened this issue Jan 14, 2020 · 8 comments · Fixed by #701
Closed

Add support for jaeger propagation formats #696

vladislav-kiva opened this issue Jan 14, 2020 · 8 comments · Fixed by #701
Assignees
Milestone

Comments

@vladislav-kiva
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Problem: In our company we have an existing ecosystem of around 50 microservices per domain. Practically all instrumented with existing jaeger libraries and they are using jaeger standards like uber-trace-id HTTP header and so on.

Describe the solution you'd like
I think we should implement smth like JaegerHttpTraceFormat.ts near B3Format.ts and HttpTraceContext.ts. That would allow us to work with jaeger format's and we can switch between formats via configuration

@vmarchaud vmarchaud changed the title Add support for jaeger formats Add support for jaeger propagation formats Jan 14, 2020
@OlivierAlbertini
Copy link
Member

If I understand correctly, you can do it like:

  1. Implementing HttpTextFormat interface.
  2. passing the concrete class to the NodeTracer config like :
new NodeTracer({
httpTextFormat: new MyCustomHttpTextFormat()
})

JaegerHttpTraceFormat is already very close to HttpTextFormat interface

@dyladan
Copy link
Member

dyladan commented Jan 14, 2020

HttpTextFormat interface is just an interface that your formatter must conform to. There is a text format and a binary format interface. You should implement the text format interface. I think @OlivierAlbertini was referring to the HttpTraceContext which is very close to the Jaeger format.

@dyladan
Copy link
Member

dyladan commented Jan 14, 2020

I removed the target label because this could be used for both web and node.

@mayurkale22
Copy link
Member

@dyladan
Copy link
Member

dyladan commented Jan 14, 2020

@vladislav-kiva are you implementing this?

@mayurkale22 mayurkale22 added this to the Alpha v0.4 milestone Jan 14, 2020
@vmarchaud
Copy link
Member

vmarchaud commented Jan 14, 2020

@dyladan He wrote on the node gitter that he started to work on that, hence i assigned it to him

@vladislav-kiva
Copy link
Contributor Author

@dyladan yeap for now i already wrote it and now i'm writing tests|

@vladislav-kiva
Copy link
Contributor Author

@mayurkale22 thanks i will take a look on it, for now i was using https://github.com/jaegertracing/jaeger-client-node

DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
DotSpy pushed a commit to DotSpy/opentelemetry-js that referenced this issue Jan 16, 2020
@mayurkale22 mayurkale22 modified the milestones: Alpha v0.4, Alpha v0.3.3 Jan 17, 2020
mayurkale22 pushed a commit that referenced this issue Jan 20, 2020
* feat: add jaeger http trace format (#696)

* feat: add jaeger http trace format (#696)

* feat: add jaeger http trace format (#696)

* feat: add jaeger http trace format (#696)

* feat: add jaeger http trace format (#696)

* feat: add jaeger http trace format (#696)

* fix: we should set sampled\unsampled via flag

* fix: we should set sampled\unsampled via flag

* fix: flags should be converted to hex, not decimal

* feat: create new package for propagation jaeger

* fix: remove unused dependencies, correct readme header, moved out jaeger from core index.ts

* fix: added jaeger keyword

* fix: remove comma

* docs: replace NodeTracer with NodeTracerRegistry

* fix: added missing jaeger keyword to exporter-jaeger

* fix: remove test for browser

* fix: remove yarn for browser

* fix: use same naming style as other packages

* feat: added index.ts and version.ts, revert test for browser

* fix: tests added index-webpack.ts

* test: add test with span generated by jaeger client

* fix: apply review changes

* fix: move out from sub dirs

* docs: use common language for docs

* fix: test script fix

Co-authored-by: Uladzislau Kiva <[email protected]>
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
…ry#701)

* feat: add jaeger http trace format (open-telemetry#696)

* feat: add jaeger http trace format (open-telemetry#696)

* feat: add jaeger http trace format (open-telemetry#696)

* feat: add jaeger http trace format (open-telemetry#696)

* feat: add jaeger http trace format (open-telemetry#696)

* feat: add jaeger http trace format (open-telemetry#696)

* fix: we should set sampled\unsampled via flag

* fix: we should set sampled\unsampled via flag

* fix: flags should be converted to hex, not decimal

* feat: create new package for propagation jaeger

* fix: remove unused dependencies, correct readme header, moved out jaeger from core index.ts

* fix: added jaeger keyword

* fix: remove comma

* docs: replace NodeTracer with NodeTracerRegistry

* fix: added missing jaeger keyword to exporter-jaeger

* fix: remove test for browser

* fix: remove yarn for browser

* fix: use same naming style as other packages

* feat: added index.ts and version.ts, revert test for browser

* fix: tests added index-webpack.ts

* test: add test with span generated by jaeger client

* fix: apply review changes

* fix: move out from sub dirs

* docs: use common language for docs

* fix: test script fix

Co-authored-by: Uladzislau Kiva <[email protected]>
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
martinkuba pushed a commit to martinkuba/opentelemetry-js that referenced this issue Mar 13, 2024
martinkuba pushed a commit to martinkuba/opentelemetry-js that referenced this issue Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants