Add telemetry events to async middleware #1169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce telemetry events to Async middleware to instrument new `Task.async/1` created inside the middleware. The use-case for this event comes from OpenTelemetry and trace propagation. For OTel, we need to run instrumentation from inside the newly created Task to be able to connect spans. With this event in place, we can track those Tasks without introducing custom wrappers on user code. This patch adds events only in the case of the middleware creating the task. For tasks created by user code, there is nothing the library can do, unfortunately.
andrewhr
commented
May 7, 2022
| @@ -75,12 +75,39 @@ defmodule Absinthe.Middleware.AsyncTest do | |||
| assert {:ok, %{data: %{"asyncBareThing" => "bare task"}}} == Absinthe.run(doc, Schema) | |||
| end | |||
|
|
|||
| test "can resolve a field using the normal test helper" do | |||
| test "can resolve a field using the normal test helper and emit telemetry event", %{test: test} do | |||
There was a problem hiding this comment.
I extended this existing test with extra telemetry. I'm happy to change to a separate test OR add telemetry assertions on the existing ones - when applicable.
|
Great addition, thanks! |
andrewhr
added a commit
to andrewhr/absinthe
that referenced
this pull request
May 9, 2022
With a similar rationale from [this PR][1], tweak the telemetry events from batch middleware. The existing event contract stays the same, but move to run from inside the Task used for batch execution. There is a small semantic difference of when the :stop event fires - this patch sends the event at the end of the task, while the current form may have a delay/sync caused by Task await. A complementary event is added as well to announce when the Resolver continuation - the post batch - happens. Given that piece is part of the resolver, the resolution struct is included as part of the metadata. [1]: absinthe-graphql#1169
andrewhr
added a commit
to andrewhr/opentelemetry-erlang-contrib
that referenced
this pull request
May 27, 2022
Add instrumentation for `Absinthe`, the GraphQL Elixir-framwork. Relies on
some new telemetry work on Absinthe:
* [Added telemetry for async helper][1], used to propagate traces
* [Improved telemetry for batch helper][2], used to propagate traces
* [Tweaked pipeline instrumentation][3], that splits parse/validate/execute
phases, following the approach of [JavaScript libraries][4]
Span names and attributes also follow the [JavaScript impl][4], the
closes to a future [Semantic Attribute][5] spec.
The "span tracker" used here is to connect traces in a shape that
resembles the query shape instead of execution shape. One example is:
```
query {
users {
name
}
}
```
In case `name` is an async resolver (for example), the execution will
resembles a recursive function, and such our trace will be:
```
RootQueryType.users
User.name
User.name
User.name
```
With the span tracker, the above trace will become:
```
RootQueryType.users
User.name
User.name
User.name
```
That helps visually scan the traces, and it's friendly to folks who
doesn't know much about Absinthe internals.
[1]: absinthe-graphql/absinthe#1169
[2]: absinthe-graphql/absinthe#1170
[3]: absinthe-graphql/absinthe#1172
[4]: https://www.npmjs.com/package/@opentelemetry/instrumentation-graphql
[5]: open-telemetry/opentelemetry-specification#2456
andrewhr
added a commit
to andrewhr/absinthe
that referenced
this pull request
Jun 15, 2022
With a similar rationale from [this PR][1], tweak the telemetry events from batch middleware. The existing event contract stays the same, but move to run from inside the Task used for batch execution. There is a small semantic difference of when the :stop event fires - this patch sends the event at the end of the task, while the current form may have a delay/sync caused by Task await. A complementary event is added as well to announce when the Resolver continuation - the post batch - happens. Given that piece is part of the resolver, the resolution struct is included as part of the metadata. [1]: absinthe-graphql#1169
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce telemetry events to Async middleware to instrument new
Task.async/1created inside the middleware.The use-case for this event comes from OpenTelemetry and trace propagation. For OTel, we need to run instrumentation from inside the newly created Task to be able to connect spans. With this event in place, we can track those Tasks without introducing custom wrappers on user code.
This patch adds events only in the case of the middleware creating the task. For tasks created by user code, there is nothing the library can do, unfortunately.