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

Support multiple span processors #43

Merged
merged 1 commit into from Aug 27, 2023

Conversation

plantfansam
Copy link
Contributor

This PR attempts to close #42. Not sure if you want this upstream @evanderkoogh, but I wanted to prove to myself it could be done, so here it is! Some demo code:

import { instrument, ResolveConfigFn } from '@microlabs/otel-cf-workers'
import { trace, Span, Context } from '@opentelemetry/api'
import { ConsoleSpanExporter, ReadableSpan, SpanProcessor } from '@opentelemetry/sdk-trace-base'

class LoggingSpanProcessor implements SpanProcessor {
	onStart(span: Span, _parentContext: Context): void {
		console.log(`startspan:${span.spanContext().spanId}`)
	}

	onEnd(span: ReadableSpan): void {
		console.log(`endspan:${span.spanContext().spanId}`)
	}

	forceFlush(): Promise<void> {
		return Promise.resolve()
	}

	shutdown(): Promise<void> {
		return Promise.resolve()
	}
}

const ResolveTraceConfigFn: ResolveConfigFn = () => {
	return {
		exporter: new ConsoleSpanExporter,
		service: { name: 'greetings' },
		spanProcessors: [new LoggingSpanProcessor]
	}
}

const handler = {
	async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
		await fetch('https://cloudflare.com')
		const greeting = "G'day World"
		trace.getActiveSpan()?.setAttribute('greeting', greeting)
		ctx.waitUntil(fetch('https://workers.dev'))
		return new Response(`${greeting}!`)
	},
}

export default instrument(handler, ResolveTraceConfigFn)

@evanderkoogh
Copy link
Owner

As we talked about on the call, I have certainly no objections to being able to use different SpanProcessors, but do you think there is a reason to have multiple SpanProcessors? Would you not either use the default one, or a different one?

@evanderkoogh evanderkoogh merged commit a14820b into evanderkoogh:main Aug 27, 2023
@plantfansam
Copy link
Contributor Author

plantfansam commented Sep 4, 2023

That's such a good question, and it sent me back down the spec rabbit hole. Re-reading the spec, I think that the key line is:

Each processor registered on TracerProvider is a start of pipeline that consist of span processor and optional exporter. SDK MUST allow to end each pipeline with individual exporter.

I think this means that a single OpenTelemetry SDK can e.g. export to different endpoints by registering multiple processor -> exporter couplets.

@evanderkoogh
Copy link
Owner

I fail to see the reason why you would want to have a different SpanProcessor for every Exporter?

@plantfansam
Copy link
Contributor Author

Yeah, to be honest, I have a hard time understanding the spec's perspective on this, and there seems to have been a lot of discussion on the topic (e.g. https://github.com/open-telemetry/opentelemetry-specification/pull/338/files) . I (very weakly) believe that multiple processors for each exporter would provide a more on-spec way of achieving this. See, e.g., having a zPages processor and a Batch Span processor that gets to an exporter in this example.

My specific use case for this library doesn't involve two export paths, just monkeying with the spans a bit when they start and end.

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

Successfully merging this pull request may close these issues.

Support for multiple span processors?
2 participants