search

open-telemetry / opentelemetry-js

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

Lighter-weight no-op API packaging #3311

Open kcirone opened 3 years ago

kcirone commented 3 years ago

There is currently no easy way to get the basic OpenTelemetry constructs - tracer, span, context - without uptaking the whole API tree including diag logging, etc. For implementations where No-Op is the default but conditional, it would be beneficial to have a lighter-weight API structure that minimizes footprint.

kcirone commented 3 years ago

Sorry closed accidentally.

dyladan commented 3 years ago

Curious what your usecase is or what you would expect to see here. Are you asking for the API and the Noop components to be separate packages? Are you using any sort of minification or tree shaking in your build?

kcirone commented 3 years ago

The issue is that if you have a framework that includes the NoOp implementation with the understanding that an app that consumes the framework will add a reference to some OT implementation as they see fit, the framework still has to drag in a lot of class dependencies in the lib that it really doesn't directly need (diag classes for instance). The OT implementation of course might need this stuff, but not a framework that essentially just wants to get a Tracer and create some Spans.

K

From: Daniel Dyla @.> Sent: Friday, August 27, 2021 1:01 PM To: open-telemetry/opentelemetry-js-api @.> Cc: Kevin Cirone @.>; State change @.> Subject: [External] : Re: [open-telemetry/opentelemetry-js-api] Lighter-weight no-op API packaging (#114)

Curious what your usecase is or what you would expect to see here. Are you asking for the API and the Noop components to be separate packages? Are you using any sort of minification or tree shaking in your build?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/open-telemetry/opentelemetry-js/issues/3311*issuecomment-907343662__;Iw!!ACWV5N9M2RV99hQ!YseNDSstV-L6pLapz5F_aZVF2KZK1Rak6r8pGAGg1MrG86OOsCdqIeJkI0kqv2GXdw$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AGKZT7XHNQF7SSIJSRUQ5LTT67AGNANCNFSM5B4UDNUQ__;!!ACWV5N9M2RV99hQ!YseNDSstV-L6pLapz5F_aZVF2KZK1Rak6r8pGAGg1MrG86OOsCdqIeJkI0mdvHMZZQ$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!ACWV5N9M2RV99hQ!YseNDSstV-L6pLapz5F_aZVF2KZK1Rak6r8pGAGg1MrG86OOsCdqIeJkI0npcJoUYg$ or Androidhttps://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!ACWV5N9M2RV99hQ!YseNDSstV-L6pLapz5F_aZVF2KZK1Rak6r8pGAGg1MrG86OOsCdqIeJkI0nPNqeyRQ$.

dyladan commented 3 years ago

Hmm. this is quite a large change and I can't think of a way to easily do it in a backwards compatible way. Off the tp of my head I can't think of anything aside from the diag classes which are not directly needed by the external API interface. Even the diag is required in order to debug the API as there is no other good way to get visibility into issues with the initial API setup process.

AlexGS74 commented 2 years ago

This would be hugely beneficial for unit testing the instrumented code in apps that have the SDK connected and loaded. With NoOps made public that would be easy to use NoopTracer etc in case if this is a test context. Or it tries to post the spans nobody needs to the backends. Usually it fails with cryptic errors instead failing totally correct tests cases.

dyladan commented 2 years ago

@AlexGS74 I think the original request is actually the opposite of what you're saying. You are asking for the no-ops to be made public, where the original request is to provide an API which doesn't include the no-ops.

@kcirone I'm transferring this to the main repo since the API is moved there now, but I'm not sure if this is a possible request. The specification states that the API MUST no-op if a method is called when there is no SDK registered. If we provide an API with no no-op behavior and no SDK is registered, we break specification. Not sure if there is any way around that or if I misunderstood the request?

github-actions[bot] commented 1 month 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.