Skip to content

Add Broadway instrumentation library #92

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andrewhr
Copy link
Contributor

@andrewhr andrewhr commented Jun 9, 2022

Library to instrument Broadway topologies, including trace linking.

Broadway is a tricky one to describe in OpenTelemetry. It feels like a good one to follow Messaging semantics (like Oban), though they're doesn't plainly map to it.

The approach taken here are usage of custom attributes to have a good start, while waiting for those Messaging semantics to stabilize - looks like they will change a lot anyway in some near future.

One alternative is to model each GenStage as it's own destination, sort-of.

Another experiment done in this patch is adding some means to propagate traces as links. Broadway doesn't give great means to do without some friction. The approach here is to add a wrapper for Broadway.start_link/2 to inject extra configuration that will take care of this context extraction and propagation.

It relies on Producer transformers and peek on some implementation details of official Producers (all them have some
behaviour-based clients we can wrap, but not exactly public API). The good thing is this is all optional - just required if span linking is desired.

Library to instrument Broadway topologies, including trace linking.

Broadway is a tricky one to describe in OpenTelemetry. It feels like a
good one to follow Messaging semantics (like Oban), though they're
doesn't plainly map to it.

The approach taken here are usage of custom attributes to have a good
start, while waiting for those Messaging semantics to stabilize - looks
like they will change a lot anyway [in some near future][1].

One alternative is to model each GenStage as it's own destination,
sort-of.

Another experiement done in this patch is adding some means to propagate
traces as links. Broadway doesn't give great means to do without
some friction. The approach here is to add a wrapper for
`Broadway.start_link/2` to inject extra configuration that will take
care of this context extraction and propagation.

It relies on Producer [`transformers`][2] and peek on some
implementation details of official Producers (all them have some
behaviour-based clients we can wrap, but not exactly public API). The
good thing is this is all optional - just required if span linking is
desired.

[1]: open-telemetry/oteps#192
[2]: https://hexdocs.pm/broadway/1.0.3/custom-producers.html
@andrewhr
Copy link
Contributor Author

andrewhr commented Jun 9, 2022

For the record: while a lot of this is experimental in the sense attributes, methods of propagation etc are WIP, a similar impl is in prod.

@tielur
Copy link

tielur commented Aug 9, 2023

Any movement on this? Is this blocked by anything? Would love to get open telemetry tracing with broadway

@tsloughter
Copy link
Member

@andrewhr any update?

@andrewhr
Copy link
Contributor Author

@tsloughter @tielur sorry for the slow answer. I plan to revisit this implementation this week, as I do need in my current job - mostly keep up to date and give a quick test on a real app.

The caveat @tsloughter is the name has been captured in the meantime. @tomtaylor has been active here, so maybe he can transfer and we could overwrite with a new implementation? I would say what we have in this PR is more complete and deals with span link / propagation so it's worth :)

@tsloughter
Copy link
Member

Great.

And that'd be great if @tomtaylor agrees?

@tomtaylor
Copy link
Contributor

Hi folks - very happy to give this library name over if there's a more complete implementation. I've not properly looked at what @andrewhr has implemented, but on the surface it looks more complete, so let's do it. What's the best way to do that?

@tielur
Copy link

tielur commented Sep 14, 2023

Looks like we had some good movement on this about 2 weeks ago, I'd love to see this make it to the finish line. Does anyone have steps to take to get this working.

Do we need this PR merge prior to the library name transfer?

@tsloughter
Copy link
Member

@tielur name transfer can wait until after merge.

Just waiting on @andrewhr to pull in updates from main and mark this as ready instead of draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants