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

feat: map OTEL's span.status: Unset to event.outcome: success #188

Merged

Conversation

kyungeunni
Copy link
Contributor

apm-data translates Span.status: Unset as event.outcome: unknown. For OTEL, Unset status implies has not been failed/errored. We'd like to map Unset to success so we can calculate the correct transaction failed rate.

The OTel specification writes the following on the SpanStatus being Unset:

When an exception is handled, a Span status can be set to Error. Otherwise, a Span status is in the Unset state. By setting a Span status to Unset, the back-end that processes spans can now assign a final status.

So it should be ok for us map the Unset to success when we process the data.

@kyungeunni kyungeunni requested a review from a team as a code owner December 4, 2023 03:22
@kyungeunni kyungeunni self-assigned this Dec 4, 2023
Copy link
Contributor

@lahsivjar lahsivjar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I am missing something here, should we be overriding explicitly set event.outcome or transaction.result?

input/otlp/traces.go Outdated Show resolved Hide resolved
input/otlp/traces.go Outdated Show resolved Hide resolved
input/otlp/traces.go Outdated Show resolved Hide resolved
@@ -174,9 +174,16 @@ func TestDecodeMapToSpanModel(t *testing.T) {
assert.Equal(t, "failure", out.Event.Outcome)
// derive from other fields - unknown
input.Outcome.Reset()
input.OTel.Reset()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was only made to OTel input for now, so I needed to reset the Otel attributes here so we don't get confused why it affects also elasticapm input

input.Context.HTTP.StatusCode.Reset()
mapToSpanModel(&input, &out)
assert.Equal(t, "unknown", out.Event.Outcome)
// outcome is success when not assigned and it's otel
input.Outcome.Reset()
input.OTel.SpanKind.Set(spanKindInternal)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was only made to OTel input for now, if otel attributes are set, it will use the same logic for mapping the Unset status to success outcome

@kyungeunni kyungeunni requested review from lahsivjar and a team December 8, 2023 06:25
@kyungeunni kyungeunni enabled auto-merge (squash) December 12, 2023 04:15
@kyungeunni kyungeunni merged commit b2a4dab into elastic:main Dec 12, 2023
4 checks passed
@kyungeunni kyungeunni deleted the otlp-set-success-outcome-for-unset branch December 12, 2023 05:19
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.

3 participants