From 86101e9e10b56aa82e01a89f9d4517a0ce4a5de6 Mon Sep 17 00:00:00 2001 From: kyungeunni Date: Mon, 4 Dec 2023 11:50:47 +0900 Subject: [PATCH 1/3] feat: set success outcome for Unset status --- .../jaeger_sampling_rate.approved.json | 10 ++++++---- .../metadata_jaeger-no-language.approved.json | 3 ++- .../metadata_jaeger-version.approved.json | 3 ++- .../test_approved/metadata_jaeger.approved.json | 3 ++- .../test_approved/span_jaeger_custom.approved.json | 2 +- .../otlp/test_approved/span_jaeger_db.approved.json | 2 +- .../span_jaeger_https_default_port.approved.json | 2 +- .../span_jaeger_messaging.approved.json | 2 +- .../span_jaeger_subtype_component.approved.json | 2 +- .../transaction_jaeger_custom.approved.json | 3 ++- .../transaction_jaeger_type_component.approved.json | 3 ++- .../transaction_jaeger_type_messaging.approved.json | 3 ++- input/otlp/traces.go | 13 ++++++++----- input/otlp/traces_test.go | 2 +- 14 files changed, 32 insertions(+), 21 deletions(-) diff --git a/input/otlp/test_approved/jaeger_sampling_rate.approved.json b/input/otlp/test_approved/jaeger_sampling_rate.approved.json index 140a7ced..636cc749 100644 --- a/input/otlp/test_approved/jaeger_sampling_rate.approved.json +++ b/input/otlp/test_approved/jaeger_sampling_rate.approved.json @@ -8,7 +8,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" @@ -24,6 +24,7 @@ }, "transaction": { "representative_count": 1.25, + "result": "Success", "sampled": true, "type": "unknown" } @@ -36,7 +37,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" @@ -69,7 +70,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" @@ -91,6 +92,7 @@ }, "transaction": { "sampled": true, + "result": "Success", "type": "unknown" } }, @@ -102,7 +104,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" diff --git a/input/otlp/test_approved/metadata_jaeger-no-language.approved.json b/input/otlp/test_approved/metadata_jaeger-no-language.approved.json index 4b4649f6..1cc29194 100644 --- a/input/otlp/test_approved/metadata_jaeger-no-language.approved.json +++ b/input/otlp/test_approved/metadata_jaeger-no-language.approved.json @@ -7,7 +7,7 @@ "version": "3.4.12" }, "event": { - "outcome": "unknown" + "outcome": "success" }, "service": { "language": { @@ -28,6 +28,7 @@ "id": "0000000041414646", "representative_count": 1, "sampled": true, + "result": "Success", "type": "unknown" } } diff --git a/input/otlp/test_approved/metadata_jaeger-version.approved.json b/input/otlp/test_approved/metadata_jaeger-version.approved.json index be4cd67c..fb933ac8 100644 --- a/input/otlp/test_approved/metadata_jaeger-version.approved.json +++ b/input/otlp/test_approved/metadata_jaeger-version.approved.json @@ -7,7 +7,7 @@ "version": "3.4.12" }, "event": { - "outcome": "unknown" + "outcome": "success" }, "service": { "language": { @@ -28,6 +28,7 @@ "id": "0000000041414646", "representative_count": 1, "sampled": true, + "result": "Success", "type": "unknown" } } diff --git a/input/otlp/test_approved/metadata_jaeger.approved.json b/input/otlp/test_approved/metadata_jaeger.approved.json index 89494312..62201de1 100644 --- a/input/otlp/test_approved/metadata_jaeger.approved.json +++ b/input/otlp/test_approved/metadata_jaeger.approved.json @@ -8,7 +8,7 @@ "version": "3.2.1" }, "event": { - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-foo", @@ -39,6 +39,7 @@ "id": "0000000041414646", "representative_count": 1, "sampled": true, + "result": "Success", "type": "unknown" } } diff --git a/input/otlp/test_approved/span_jaeger_custom.approved.json b/input/otlp/test_approved/span_jaeger_custom.approved.json index 95d6ff8b..6c387339 100644 --- a/input/otlp/test_approved/span_jaeger_custom.approved.json +++ b/input/otlp/test_approved/span_jaeger_custom.approved.json @@ -8,7 +8,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" diff --git a/input/otlp/test_approved/span_jaeger_db.approved.json b/input/otlp/test_approved/span_jaeger_db.approved.json index 5bba422b..570e8303 100644 --- a/input/otlp/test_approved/span_jaeger_db.approved.json +++ b/input/otlp/test_approved/span_jaeger_db.approved.json @@ -12,7 +12,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" diff --git a/input/otlp/test_approved/span_jaeger_https_default_port.approved.json b/input/otlp/test_approved/span_jaeger_https_default_port.approved.json index 5b82b4cf..bc5ce442 100644 --- a/input/otlp/test_approved/span_jaeger_https_default_port.approved.json +++ b/input/otlp/test_approved/span_jaeger_https_default_port.approved.json @@ -12,7 +12,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" diff --git a/input/otlp/test_approved/span_jaeger_messaging.approved.json b/input/otlp/test_approved/span_jaeger_messaging.approved.json index df4c74f7..fa347182 100644 --- a/input/otlp/test_approved/span_jaeger_messaging.approved.json +++ b/input/otlp/test_approved/span_jaeger_messaging.approved.json @@ -12,7 +12,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" diff --git a/input/otlp/test_approved/span_jaeger_subtype_component.approved.json b/input/otlp/test_approved/span_jaeger_subtype_component.approved.json index 204f7a00..b38b94c4 100644 --- a/input/otlp/test_approved/span_jaeger_subtype_component.approved.json +++ b/input/otlp/test_approved/span_jaeger_subtype_component.approved.json @@ -8,7 +8,7 @@ }, "event": { "duration": 79000000000, - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" diff --git a/input/otlp/test_approved/transaction_jaeger_custom.approved.json b/input/otlp/test_approved/transaction_jaeger_custom.approved.json index ce6650b3..0a73ecc0 100644 --- a/input/otlp/test_approved/transaction_jaeger_custom.approved.json +++ b/input/otlp/test_approved/transaction_jaeger_custom.approved.json @@ -7,7 +7,7 @@ "version": "unknown" }, "event": { - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" @@ -27,6 +27,7 @@ "transaction": { "representative_count": 1, "sampled": true, + "result": "Success", "type": "unknown" } } diff --git a/input/otlp/test_approved/transaction_jaeger_type_component.approved.json b/input/otlp/test_approved/transaction_jaeger_type_component.approved.json index 67e8638a..439aa67f 100644 --- a/input/otlp/test_approved/transaction_jaeger_type_component.approved.json +++ b/input/otlp/test_approved/transaction_jaeger_type_component.approved.json @@ -7,7 +7,7 @@ "version": "unknown" }, "event": { - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" @@ -27,6 +27,7 @@ "transaction": { "representative_count": 1, "sampled": true, + "result": "Success", "type": "unknown" } } diff --git a/input/otlp/test_approved/transaction_jaeger_type_messaging.approved.json b/input/otlp/test_approved/transaction_jaeger_type_messaging.approved.json index 6e07198d..d48f53fd 100644 --- a/input/otlp/test_approved/transaction_jaeger_type_messaging.approved.json +++ b/input/otlp/test_approved/transaction_jaeger_type_messaging.approved.json @@ -7,7 +7,7 @@ "version": "unknown" }, "event": { - "outcome": "unknown" + "outcome": "success" }, "host": { "hostname": "host-abc" @@ -31,6 +31,7 @@ } }, "representative_count": 1, + "result": "Success", "sampled": true, "type": "messaging" } diff --git a/input/otlp/traces.go b/input/otlp/traces.go index f51910da..9248eb75 100644 --- a/input/otlp/traces.go +++ b/input/otlp/traces.go @@ -468,11 +468,12 @@ func TranslateTransaction( } // Set outcome nad result from status code. + // However, if outcome was explicitly set to failure, preserve it if statusCode := httpResponse.StatusCode; statusCode > 0 { - if event.Event.Outcome == outcomeUnknown { + if event.Event.Outcome != outcomeFailure { event.Event.Outcome = serverHTTPStatusCodeOutcome(int(statusCode)) } - if event.Transaction.Result == "" { + if event.Transaction.Result != "Error" { event.Transaction.Result = httpStatusCodeResult(int(statusCode)) } } @@ -815,7 +816,7 @@ func TranslateSpan(spanKind ptrace.SpanKind, attributes pcommon.Map, event *mode } if isHTTP { - if httpResponse.StatusCode > 0 && event.Event.Outcome == outcomeUnknown { + if httpResponse.StatusCode > 0 { event.Event.Outcome = clientHTTPStatusCodeOutcome(int(httpResponse.StatusCode)) } if http.SizeVT() != 0 { @@ -1183,9 +1184,11 @@ func replaceDots(s string) string { // spanStatusOutcome returns the outcome for transactions and spans based on // the given OTLP span status. +// we translate status into success in case it is not ERROR +// reference: https://opentelemetry.io/docs/concepts/signals/traces/#span-status func spanStatusOutcome(status ptrace.Status) string { switch status.Code() { - case ptrace.StatusCodeOk: + case ptrace.StatusCodeOk, ptrace.StatusCodeUnset: return outcomeSuccess case ptrace.StatusCodeError: return outcomeFailure @@ -1198,7 +1201,7 @@ func spanStatusOutcome(status ptrace.Status) string { // is returned. func spanStatusResult(status ptrace.Status) string { switch status.Code() { - case ptrace.StatusCodeOk: + case ptrace.StatusCodeOk, ptrace.StatusCodeUnset: return "Success" case ptrace.StatusCodeError: return "Error" diff --git a/input/otlp/traces_test.go b/input/otlp/traces_test.go index a38f7ae7..c7951314 100644 --- a/input/otlp/traces_test.go +++ b/input/otlp/traces_test.go @@ -106,7 +106,7 @@ func TestOutcome(t *testing.T) { assert.Equal(t, expectedOutcome, (*batch)[1].GetEvent().GetOutcome()) } - test(t, "unknown", "", ptrace.StatusCodeUnset) + test(t, "success", "Success", ptrace.StatusCodeUnset) test(t, "success", "Success", ptrace.StatusCodeOk) test(t, "failure", "Error", ptrace.StatusCodeError) } From 3fc1ddb3c89ae0e691fba76f9ff798882d684a38 Mon Sep 17 00:00:00 2001 From: kyungeunni Date: Tue, 5 Dec 2023 12:38:38 +0900 Subject: [PATCH 2/3] chore: do not assign success as default and check it at the last stage --- .../internal/modeldecoder/v2/span_test.go | 8 +++++- .../modeldecoder/v2/transaction_test.go | 7 +++++ input/otlp/traces.go | 26 +++++++++++++------ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/input/elasticapm/internal/modeldecoder/v2/span_test.go b/input/elasticapm/internal/modeldecoder/v2/span_test.go index 82ed6852..74b22824 100644 --- a/input/elasticapm/internal/modeldecoder/v2/span_test.go +++ b/input/elasticapm/internal/modeldecoder/v2/span_test.go @@ -172,8 +172,14 @@ func TestDecodeMapToSpanModel(t *testing.T) { input.Context.HTTP.StatusCode.Set(http.StatusBadRequest) mapToSpanModel(&input, &out) assert.Equal(t, "failure", out.Event.Outcome) - // derive from other fields - unknown + // derive from other fields - success + input.Outcome.Reset() + input.Context.HTTP.StatusCode.Reset() + mapToSpanModel(&input, &out) + assert.Equal(t, "success", out.Event.Outcome) + // derive from other fields - unknown when not otel input.Outcome.Reset() + input.OTel.Reset() input.Context.HTTP.StatusCode.Reset() mapToSpanModel(&input, &out) assert.Equal(t, "unknown", out.Event.Outcome) diff --git a/input/elasticapm/internal/modeldecoder/v2/transaction_test.go b/input/elasticapm/internal/modeldecoder/v2/transaction_test.go index 03899501..4c4080bf 100644 --- a/input/elasticapm/internal/modeldecoder/v2/transaction_test.go +++ b/input/elasticapm/internal/modeldecoder/v2/transaction_test.go @@ -421,8 +421,15 @@ func TestDecodeMapToTransactionModel(t *testing.T) { input.Context.Response.StatusCode.Set(http.StatusInternalServerError) mapToTransactionModel(&input, &out) assert.Equal(t, "failure", out.Event.Outcome) + // derive from other fields - if unset, treat it as success + input.Outcome.Reset() + input.Context.Response.StatusCode.Reset() + mapToTransactionModel(&input, &out) + assert.Equal(t, "success", out.Event.Outcome) + // derive from other fields - unknown input.Outcome.Reset() + input.OTel.Reset() input.Context.Response.StatusCode.Reset() mapToTransactionModel(&input, &out) assert.Equal(t, "unknown", out.Event.Outcome) diff --git a/input/otlp/traces.go b/input/otlp/traces.go index 9248eb75..b6c6bd3d 100644 --- a/input/otlp/traces.go +++ b/input/otlp/traces.go @@ -468,12 +468,11 @@ func TranslateTransaction( } // Set outcome nad result from status code. - // However, if outcome was explicitly set to failure, preserve it if statusCode := httpResponse.StatusCode; statusCode > 0 { - if event.Event.Outcome != outcomeFailure { + if event.Event.Outcome == outcomeUnknown { event.Event.Outcome = serverHTTPStatusCodeOutcome(int(statusCode)) } - if event.Transaction.Result != "Error" { + if event.Transaction.Result == "" { event.Transaction.Result = httpStatusCodeResult(int(statusCode)) } } @@ -521,6 +520,14 @@ func TranslateTransaction( event.Service.Framework.Name = name event.Service.Framework.Version = library.Version() } + + // if outcome and result are still not assigned, assign success + if event.Event.Outcome == outcomeUnknown { + event.Event.Outcome = outcomeSuccess + if event.Transaction.Result == "" { + event.Transaction.Result = "Success" + } + } } // TranslateSpan converts incoming otlp/otel trace data into the @@ -816,7 +823,7 @@ func TranslateSpan(spanKind ptrace.SpanKind, attributes pcommon.Map, event *mode } if isHTTP { - if httpResponse.StatusCode > 0 { + if httpResponse.StatusCode > 0 && event.Event.Outcome == outcomeUnknown { event.Event.Outcome = clientHTTPStatusCodeOutcome(int(httpResponse.StatusCode)) } if http.SizeVT() != 0 { @@ -946,6 +953,11 @@ func TranslateSpan(spanKind ptrace.SpanKind, attributes pcommon.Map, event *mode // The client has reported its sampling rate, so we can use it to extrapolate transaction metrics. parseSamplerAttributes(samplerType, samplerParam, event) } + + // if outcome is still not assigned, assign success + if event.Event.Outcome == outcomeUnknown { + event.Event.Outcome = outcomeSuccess + } } func parseSamplerAttributes(samplerType, samplerParam pcommon.Value, event *modelpb.APMEvent) { @@ -1184,11 +1196,9 @@ func replaceDots(s string) string { // spanStatusOutcome returns the outcome for transactions and spans based on // the given OTLP span status. -// we translate status into success in case it is not ERROR -// reference: https://opentelemetry.io/docs/concepts/signals/traces/#span-status func spanStatusOutcome(status ptrace.Status) string { switch status.Code() { - case ptrace.StatusCodeOk, ptrace.StatusCodeUnset: + case ptrace.StatusCodeOk: return outcomeSuccess case ptrace.StatusCodeError: return outcomeFailure @@ -1201,7 +1211,7 @@ func spanStatusOutcome(status ptrace.Status) string { // is returned. func spanStatusResult(status ptrace.Status) string { switch status.Code() { - case ptrace.StatusCodeOk, ptrace.StatusCodeUnset: + case ptrace.StatusCodeOk: return "Success" case ptrace.StatusCodeError: return "Error" From 0622c40a8fa84bea444062bd01f6d54cc0db3568 Mon Sep 17 00:00:00 2001 From: kyungeunni Date: Tue, 5 Dec 2023 12:46:11 +0900 Subject: [PATCH 3/3] chore: align tests --- .../elasticapm/internal/modeldecoder/v2/span_test.go | 11 ++++++----- .../internal/modeldecoder/v2/transaction_test.go | 12 ++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/input/elasticapm/internal/modeldecoder/v2/span_test.go b/input/elasticapm/internal/modeldecoder/v2/span_test.go index 74b22824..520bc923 100644 --- a/input/elasticapm/internal/modeldecoder/v2/span_test.go +++ b/input/elasticapm/internal/modeldecoder/v2/span_test.go @@ -172,17 +172,18 @@ func TestDecodeMapToSpanModel(t *testing.T) { input.Context.HTTP.StatusCode.Set(http.StatusBadRequest) mapToSpanModel(&input, &out) assert.Equal(t, "failure", out.Event.Outcome) - // derive from other fields - success + // derive from other fields - unknown input.Outcome.Reset() + input.OTel.Reset() input.Context.HTTP.StatusCode.Reset() mapToSpanModel(&input, &out) - assert.Equal(t, "success", out.Event.Outcome) - // derive from other fields - unknown when not otel + assert.Equal(t, "unknown", out.Event.Outcome) + // outcome is success when not assigned and it's otel input.Outcome.Reset() - input.OTel.Reset() + input.OTel.SpanKind.Set(spanKindInternal) input.Context.HTTP.StatusCode.Reset() mapToSpanModel(&input, &out) - assert.Equal(t, "unknown", out.Event.Outcome) + assert.Equal(t, "success", out.Event.Outcome) }) t.Run("timestamp", func(t *testing.T) { diff --git a/input/elasticapm/internal/modeldecoder/v2/transaction_test.go b/input/elasticapm/internal/modeldecoder/v2/transaction_test.go index 4c4080bf..662071f7 100644 --- a/input/elasticapm/internal/modeldecoder/v2/transaction_test.go +++ b/input/elasticapm/internal/modeldecoder/v2/transaction_test.go @@ -421,18 +421,18 @@ func TestDecodeMapToTransactionModel(t *testing.T) { input.Context.Response.StatusCode.Set(http.StatusInternalServerError) mapToTransactionModel(&input, &out) assert.Equal(t, "failure", out.Event.Outcome) - // derive from other fields - if unset, treat it as success - input.Outcome.Reset() - input.Context.Response.StatusCode.Reset() - mapToTransactionModel(&input, &out) - assert.Equal(t, "success", out.Event.Outcome) - // derive from other fields - unknown input.Outcome.Reset() input.OTel.Reset() input.Context.Response.StatusCode.Reset() mapToTransactionModel(&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) + input.Context.Response.StatusCode.Reset() + mapToTransactionModel(&input, &out) + assert.Equal(t, "success", out.Event.Outcome) }) t.Run("session", func(t *testing.T) {