-
Notifications
You must be signed in to change notification settings - Fork 34
Support for initializing span processors and exporters without initializing the NodeSDK #594
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
Comments
Hey @wrose504, and thanks for submitting this one! 💪 A PR will be more than welcome in general :) Let's keep in touch. |
Thanks! I should possibly describe a little more what I'm trying to do in case I'm asking for a weird solution to a non-problem 😅 We have a Hapi.js app server where a handful of our routes make AI agent calls and Traceloop is monitoring these calls because we've called Now I want to add in tracing generally for all our app routes, again using OpenTelemetry, with several other instrumentations added in (a customized Hapi instrumentation, plus Postgres and Redis instrumentations). Ideally I don't want to send all the traces for all these services and all app API calls to Traceloop. I do want to send all our Hapi/Postgres/Redis traces to GCP Cloud Trace, or some other observability platform. I can send the AI agent traces to the other observability platform too, but I won't do a lot of analysis with them there given we're using Traceloop for that. I'd also like to use GCP resource detection since we run in a GKE cluster. I'm struggling with this as while it looks like I could customize the exporter via But looking at the OpenTelemetry Node.js SDK, it seems like I should also be able to set up multiple span processors and do the same work in-process without needing to spin up the OpenTelemetry Collector. So my first thought was that I needed to be able to construct // import {TraceExporter} from '@google-cloud/opentelemetry-cloud-trace-exporter'
export const gcpTraceExporter = new TraceExporter({resourceFilter: /.*/});
export const gcpSpanProcessor = new BatchSpanProcessor(gcpTraceExporter);
// Custom sampler for sampling decisions
const sampler = new PicnicTraceSampler();
// Pretend I can import these from '@traceloop/node-server-sdk'
const traceExporter = setupTraceloopTraceExporter(config.traceloopApiKey);
const traceloopSpanProcessor = setupTraceloopSpanProcessor(traceExporter);
const sdk = new NodeSDK({
serviceName: 'picnic-app',
resourceDetectors: [
hostDetector,
gcpDetector
],
autoDetectResources: true,
spanProcessors: [
gcpSpanProcessor,
traceloopSpanProcessor
],
sampler: sampler,
instrumentations: [
// General app instrumentations
new PostgresInstrumentation(),
new RedisInstrumentation(),
new HttpInstrumentation(),
new UndiciInstrumentation(),
// Traceloop instrumentations
new OpenAIInstrumentation(),
new AzureOpenAIInstrumentation()
]
}); Where this is falling apart is that |
Initial thoughts:
Suggested Solution:Given all of the above, what I suggest, is that we'll expose in our SDK all of the capabilities of the NodeSDK, either by letting you pass the NodeSDK instance as an argument to Traceloop SDK or by exposing the same arguments exposed by the NodeSDK constructor. You'll then initialize traceloop sdk instead of the NodeSDK, with the Resource Detector and everything you with, and with two different @wrose504 LMK WDYT |
Thanks @galkleinman! I'm not completely sure I understand how I'll be able to provide a filtering span processor that can wrap Traceloop's span processor. Will there be an extra initialization option like What I have done so far to get things working is to import most of the Traceloop SDK code that works with the spans into a new file Part of the gist shows how I'm wrapping the Traceloop span processor: let traceloopSpanProcessor: SpanProcessor | undefined;
if (traceloopApiKey) {
// Setup the Traceloop trace exporter and span processor.
const traceExporter = setupTraceloopTraceExporter(traceloopApiKey);
traceloopSpanProcessor = setupTraceloopSpanProcessor(traceExporter);
// Wrap the span processor in a filter that only emits spans that are
// related to Traceloop.
traceloopSpanProcessor = new TraceloopSpanFilter(traceloopSpanProcessor);
} else {
console.warn('Traceloop API key not provided, skipping initialization');
} I think you're proposing allowing |
With a bit more work I think I'm starting to see what you meant about black magic @galkleinman 😅 I had to copy enough of the SDK that in the end it was easier to create a fork and make some edits there. Here's a diff that I hope is publicly accessible. |
Well, just saw your latest comment here @wrose504, right after I decided to take your approach and try to expose a standalone span processor 😂 Anyway, i'll keep you posted both here and on Slack. |
Currently calling
initialize
imported from@traceloop/node-server-sdk
will initialize the OpenTelemetry SDK by instantiatingNodeSDK
, configuring it, and then callingsdk.start()
. It would be good to provide a way to just get the OLTP exporter configured for Traceloop, and the batch or simple span processor linked to that exporter, so these can be added to an existing configuration that is already callingsdk.start()
.While trying to copy just the parts of the
initialize()
implementation necessary to set up the span processor, I ended up having to copy a significant chunk of code as the span processor refers to context keys (e.g.WORKFLOW_NAME_KEY
,ENTITY_NAME_KEY
) that are also not exported, meaning that I need to create copies of these keys and then make copies ofwithEntity
, etc, that use the copies.I think this could be solved by making an exported function that constructs a span processor given the
exporter
anddisableBatch
flags, or by exporting the context keys so that a span processor implementation can be built that still interacts withwithEntity
.Would a PR for either of these approaches be acceptable?
The text was updated successfully, but these errors were encountered: