search

open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.77k stars 810 forks source link

Jaeger exporter: bundling with webpack not possible. #2784

Closed AlexRRR closed 1 year ago

AlexRRR commented 2 years ago

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

"@opentelemetry/api": "^1.0.4",

What version of Node are you using?

node14

What did you do?

Deploying a serverless function to Openwhisk using the jaeger-exporter from this library.

Additional context

As discussed in https://github.com/open-telemetry/opentelemetry-js/issues/2618 jaeger-client is deprecated, however we are stuck in ^3.15.0 , I am particularly interested in the fix that got introduced in 3.18.1 which allows the client to be correctly bundled via webpack.

This is important because serverless functions usually have a hard limit on package size and bundling via webpack helps reduce it. Serverless functions are great use case for the opentelemetry/exporter-jaeger as normally in those environments there is no access to a long running OTEL collector to send traces.

Would bumping the versions to 3.18.1 be a problem? or ideally to the latest 3.19.0

dyladan commented 2 years ago

Bumping to latest is probably fine. We are going to be vendoring the code in our own repository soon too anyway

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

legendecas commented 2 years ago

Bumping to latest is probably fine. We are going to be vendoring the code in our own repository soon too anyway

@dyladan Sounds like this issue could be merged into https://github.com/open-telemetry/opentelemetry-js/issues/2618?

dyladan commented 2 years ago

It likely will be fixed by #2618 but isn't really the same issue. I can go either way.

legendecas commented 2 years ago

Tried to take another look and I found that @opentelemetry/exporter-jaeger has a semver ^3.15.0 on jaeger-client. I didn't find there is a problem to use @opentelemetry/exporter-jaeger with jaeger-client upgraded to v3.19.0 as long as the local lock files like package-lock.json is been updated.

I'm closing this as there isn't anything we need to do to fix the problem. Please feel free to re-open if there is anything new to report.

AlexRRR commented 2 years ago

@legendecas @dyladan : I wanted to document my findings, sadly just updating to the latest version of jaeger-client won't be enough, as the PR that implemented it was closed (https://github.com/jaegertracing/jaeger-client-node/issues/421) without being merged, but I can confirm that the fix works and I was able to bundle via webpack. (I've created a a fork and applied the changes)

Vendoring is not as simple as I first thought as it is not just the http and udp senders that need to be ported but also all of the thrift models. To make matters worst this is all written in flow so it would be necessary to translate to typescript which is out of my depth, but I'd would definitively love to contribute.

There are improvements that I'd like to share (eg: adding custom headers to the httpsender requests for things like auth) but without the vendoring its not possible.

pichlermarc commented 1 year ago

Closing this as "not planned" as the JaegerExporter is now deprecated and Jaeger now supports OTLP.