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

compatible sampling support #359

Open
trentm opened this issue Sep 6, 2024 · 0 comments
Open

compatible sampling support #359

trentm opened this issue Sep 6, 2024 · 0 comments

Comments

@trentm
Copy link
Member

trentm commented Sep 6, 2024

We should test whether sampling with distributed tracing works at all. Take this dev patch:

diff --git a/examples/start-elastic-sdk.js b/examples/start-elastic-sdk.js
index 99875f0..f0f0116 100644
--- a/examples/start-elastic-sdk.js
+++ b/examples/start-elastic-sdk.js
@@ -42,6 +42,9 @@ const {
     ExpressInstrumentation,
 } = require('@opentelemetry/instrumentation-express');

+const {tracing} = require('@opentelemetry/sdk-node');
+const {ParentBasedSampler, TraceIdRatioBasedSampler} = tracing;
+
 const sdk = new ElasticNodeSDK({
     serviceName: path.parse(process.argv[1]).name,
     // One can **override** completely the instrumentations provided by ElasticNodeSDK
@@ -69,6 +91,9 @@ const sdk = new ElasticNodeSDK({
         // the OTel NodeSDK, for example:
         //  new AnotherInstrumentation(),
     ],
+    sampler: new ParentBasedSampler({
+        root: new TraceIdRatioBasedSampler(0.5),
+    }),
 });
  • Should we be enabling ParentBasedSampler by default?
  • Should we offer a config envvar for selecting a sampling ratio, and then using TraceIdRatioBasedSampler if a value is given?
  • Should the upstream sdk-node being doing any of this? What does the spec say, if anything?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant