-
Notifications
You must be signed in to change notification settings - Fork 116
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
Ex aws instrumentation #330
base: main
Are you sure you want to change the base?
Conversation
… unimplemented services
… AWS returns a failed response
The committers listed above are authorized under a signed CLA. |
extract_attributes(response_body, Map.get(attribute_definition, :response, [])) | ||
end | ||
|
||
defp response_attributes(%{result: :error}, _), do: %{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't there be a Tracer.set_status()
call based on the error result?
As an example in req
:
https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/instrumentation/opentelemetry_req/lib/opentelemetry_req.ex#L108
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it depends. An error response doesn't mean what the Span is doing errored, just that the response from the backend was an error, if that makes sense...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should set error,
The tesla telemetry middleware assumes the span is an error if the response is 4xx.
Thanks @prithvihv ! |
#152