diff --git a/CHANGELOG.md b/CHANGELOG.md index 144fcdec110..c84f6cb5300 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ ## 🛑 Breaking changes 🛑 - Remove deprecated function `IsValid` from trace/span ID (#2522) +- Remove accessors for deprecated status code (#2521) + +## 🧰 Bug fixes 🧰 + +- `otlp` receiver: Sets the correct deprecated status code before sending data to the pipeline (#2521) ## v0.20.0 Beta diff --git a/cmd/pdatagen/internal/trace_structs.go b/cmd/pdatagen/internal/trace_structs.go index fd42c8b080c..3d6c5d23838 100644 --- a/cmd/pdatagen/internal/trace_structs.go +++ b/cmd/pdatagen/internal/trace_structs.go @@ -196,14 +196,6 @@ var spanStatus = &messageValueStruct{ // to OTLP spec https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231 manualSetter: true, }, - &primitiveTypedField{ - fieldName: "DeprecatedCode", - originFieldName: "DeprecatedCode", - returnType: "DeprecatedStatusCode", - rawType: "otlptrace.Status_DeprecatedStatusCode", - defaultVal: "DeprecatedStatusCode(0)", - testVal: "DeprecatedStatusCode(1)", - }, &primitiveField{ fieldName: "Message", originFieldName: "Message", diff --git a/consumer/pdata/generated_trace.go b/consumer/pdata/generated_trace.go index 6ff96ddc706..2e813dfe277 100644 --- a/consumer/pdata/generated_trace.go +++ b/consumer/pdata/generated_trace.go @@ -1011,16 +1011,6 @@ func (ms SpanStatus) Code() StatusCode { return StatusCode((*ms.orig).Code) } -// DeprecatedCode returns the deprecatedcode associated with this SpanStatus. -func (ms SpanStatus) DeprecatedCode() DeprecatedStatusCode { - return DeprecatedStatusCode((*ms.orig).DeprecatedCode) -} - -// SetDeprecatedCode replaces the deprecatedcode associated with this SpanStatus. -func (ms SpanStatus) SetDeprecatedCode(v DeprecatedStatusCode) { - (*ms.orig).DeprecatedCode = otlptrace.Status_DeprecatedStatusCode(v) -} - // Message returns the message associated with this SpanStatus. func (ms SpanStatus) Message() string { return (*ms.orig).Message @@ -1034,6 +1024,5 @@ func (ms SpanStatus) SetMessage(v string) { // CopyTo copies all properties from the current struct to the dest. func (ms SpanStatus) CopyTo(dest SpanStatus) { dest.SetCode(ms.Code()) - dest.SetDeprecatedCode(ms.DeprecatedCode()) dest.SetMessage(ms.Message()) } diff --git a/consumer/pdata/generated_trace_test.go b/consumer/pdata/generated_trace_test.go index 950e21fd85c..70439b40952 100644 --- a/consumer/pdata/generated_trace_test.go +++ b/consumer/pdata/generated_trace_test.go @@ -857,14 +857,6 @@ func TestSpanStatus_Code(t *testing.T) { assert.EqualValues(t, testValCode, ms.Code()) } -func TestSpanStatus_DeprecatedCode(t *testing.T) { - ms := NewSpanStatus() - assert.EqualValues(t, DeprecatedStatusCode(0), ms.DeprecatedCode()) - testValDeprecatedCode := DeprecatedStatusCode(1) - ms.SetDeprecatedCode(testValDeprecatedCode) - assert.EqualValues(t, testValDeprecatedCode, ms.DeprecatedCode()) -} - func TestSpanStatus_Message(t *testing.T) { ms := NewSpanStatus() assert.EqualValues(t, "", ms.Message()) @@ -1019,6 +1011,5 @@ func generateTestSpanStatus() SpanStatus { func fillTestSpanStatus(tv SpanStatus) { tv.SetCode(StatusCode(1)) - tv.SetDeprecatedCode(DeprecatedStatusCode(1)) tv.SetMessage("cancelled") } diff --git a/consumer/pdata/trace.go b/consumer/pdata/trace.go index d750fe94e01..a26fa18a7fb 100644 --- a/consumer/pdata/trace.go +++ b/consumer/pdata/trace.go @@ -121,35 +121,6 @@ const ( SpanKindCONSUMER = SpanKind(otlptrace.Span_SPAN_KIND_CONSUMER) ) -// DeprecatedStatusCode is the deprecated status code used previously. -// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status -// Deprecated: use StatusCode instead. -type DeprecatedStatusCode otlptrace.Status_DeprecatedStatusCode - -const ( - DeprecatedStatusCodeOk = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_OK) - DeprecatedStatusCodeCancelled = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_CANCELLED) - DeprecatedStatusCodeUnknownError = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR) - DeprecatedStatusCodeInvalidArgument = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_INVALID_ARGUMENT) - DeprecatedStatusCodeDeadlineExceeded = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_DEADLINE_EXCEEDED) - DeprecatedStatusCodeNotFound = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_NOT_FOUND) - DeprecatedStatusCodeAlreadyExists = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_ALREADY_EXISTS) - DeprecatedStatusCodePermissionDenied = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_PERMISSION_DENIED) - DeprecatedStatusCodeResourceExhausted = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_RESOURCE_EXHAUSTED) - DeprecatedStatusCodeFailedPrecondition = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_FAILED_PRECONDITION) - DeprecatedStatusCodeAborted = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED) - DeprecatedStatusCodeOutOfRange = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_OUT_OF_RANGE) - DeprecatedStatusCodeUnimplemented = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNIMPLEMENTED) - DeprecatedStatusCodeInternalError = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_INTERNAL_ERROR) - DeprecatedStatusCodeUnavailable = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNAVAILABLE) - DeprecatedStatusCodeDataLoss = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_DATA_LOSS) - DeprecatedStatusCodeUnauthenticated = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNAUTHENTICATED) -) - -func (sc DeprecatedStatusCode) String() string { - return otlptrace.Status_DeprecatedStatusCode(sc).String() -} - // StatusCode mirrors the codes defined at // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status type StatusCode otlptrace.Status_StatusCode @@ -166,21 +137,12 @@ func (sc StatusCode) String() string { return otlptrace.Status_StatusCode(sc).St func (ms SpanStatus) SetCode(v StatusCode) { ms.orig.Code = otlptrace.Status_StatusCode(v) - // According to OTLP spec we also need to set the deprecated_code field. - // See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231 - // - // if code==STATUS_CODE_UNSET then `deprecated_code` MUST be - // set to DEPRECATED_STATUS_CODE_OK. - // - // if code==STATUS_CODE_OK then `deprecated_code` MUST be - // set to DEPRECATED_STATUS_CODE_OK. - // - // if code==STATUS_CODE_ERROR then `deprecated_code` MUST be - // set to DEPRECATED_STATUS_CODE_UNKNOWN_ERROR. + // According to OTLP spec we also need to set the deprecated_code field as we are a new sender: + // See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L239 switch v { case StatusCodeUnset, StatusCodeOk: - ms.SetDeprecatedCode(DeprecatedStatusCodeOk) + ms.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK case StatusCodeError: - ms.SetDeprecatedCode(DeprecatedStatusCodeUnknownError) + ms.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR } } diff --git a/consumer/pdata/trace_test.go b/consumer/pdata/trace_test.go index efa8001e111..07afd4972e6 100644 --- a/consumer/pdata/trace_test.go +++ b/consumer/pdata/trace_test.go @@ -128,24 +128,21 @@ func TestSpanStatusCode(t *testing.T) { // // if code==STATUS_CODE_UNSET then `deprecated_code` MUST be // set to DEPRECATED_STATUS_CODE_OK. - status.SetDeprecatedCode(DeprecatedStatusCodeUnknownError) - assert.EqualValues(t, DeprecatedStatusCodeUnknownError, status.DeprecatedCode()) + status.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR status.SetCode(StatusCodeUnset) - assert.EqualValues(t, DeprecatedStatusCodeOk, status.DeprecatedCode()) + assert.EqualValues(t, otlptrace.Status_DEPRECATED_STATUS_CODE_OK, status.orig.DeprecatedCode) // if code==STATUS_CODE_OK then `deprecated_code` MUST be // set to DEPRECATED_STATUS_CODE_OK. - status.SetDeprecatedCode(DeprecatedStatusCodeUnknownError) - assert.EqualValues(t, DeprecatedStatusCodeUnknownError, status.DeprecatedCode()) + status.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR status.SetCode(StatusCodeOk) - assert.EqualValues(t, DeprecatedStatusCodeOk, status.DeprecatedCode()) + assert.EqualValues(t, otlptrace.Status_DEPRECATED_STATUS_CODE_OK, status.orig.DeprecatedCode) // if code==STATUS_CODE_ERROR then `deprecated_code` MUST be // set to DEPRECATED_STATUS_CODE_UNKNOWN_ERROR. - status.SetDeprecatedCode(DeprecatedStatusCodeOk) - assert.EqualValues(t, DeprecatedStatusCodeOk, status.DeprecatedCode()) + status.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK status.SetCode(StatusCodeError) - assert.EqualValues(t, DeprecatedStatusCodeUnknownError, status.DeprecatedCode()) + assert.EqualValues(t, otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, status.orig.DeprecatedCode) } func TestToFromOtlp(t *testing.T) { diff --git a/receiver/otlpreceiver/trace/otlp.go b/receiver/otlpreceiver/trace/otlp.go index 851a8275b1d..6dd2d3cf887 100644 --- a/receiver/otlpreceiver/trace/otlp.go +++ b/receiver/otlpreceiver/trace/otlp.go @@ -55,26 +55,22 @@ func (r *Receiver) Export(ctx context.Context, req *collectortrace.ExportTraceSe ctxWithReceiverName := obsreport.ReceiverContext(ctx, r.instanceName, receiverTransport) // Perform backward compatibility conversion of Span Status code according to - // OTLP specification. - // See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231 - // - // If code==STATUS_CODE_UNSET then the value of `deprecated_code` is the - // carrier of the overall status according to these rules: - // - // if deprecated_code==DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret - // the overall status to be STATUS_CODE_UNSET. - // - // if deprecated_code!=DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret - // the overall status to be STATUS_CODE_ERROR. - // - // If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be - // ignored, the `code` field is the sole carrier of the status. + // OTLP specification as we are a new receiver and sender (we are pushing data to the pipelines): + // See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L239 + // See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L253 for _, rss := range req.ResourceSpans { for _, ils := range rss.InstrumentationLibrarySpans { for _, span := range ils.Spans { - if span.Status.Code == otlptrace.Status_STATUS_CODE_UNSET && - span.Status.DeprecatedCode != otlptrace.Status_DEPRECATED_STATUS_CODE_OK { - span.Status.Code = otlptrace.Status_STATUS_CODE_ERROR + switch span.Status.Code { + case otlptrace.Status_STATUS_CODE_UNSET: + if span.Status.DeprecatedCode != otlptrace.Status_DEPRECATED_STATUS_CODE_OK { + span.Status.Code = otlptrace.Status_STATUS_CODE_ERROR + } + case otlptrace.Status_STATUS_CODE_OK: + // If status code is set then overwrites deprecated. + span.Status.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK + case otlptrace.Status_STATUS_CODE_ERROR: + span.Status.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR } } } diff --git a/receiver/otlpreceiver/trace/otlp_test.go b/receiver/otlpreceiver/trace/otlp_test.go index 350367a6aeb..657f990f723 100644 --- a/receiver/otlpreceiver/trace/otlp_test.go +++ b/receiver/otlpreceiver/trace/otlp_test.go @@ -196,9 +196,10 @@ func TestDeprecatedStatusCode(t *testing.T) { // See specification for handling status code here: // https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231 tests := []struct { - sendCode otlptrace.Status_StatusCode - sendDeprecatedCode otlptrace.Status_DeprecatedStatusCode - expectedRcvCode otlptrace.Status_StatusCode + sendCode otlptrace.Status_StatusCode + sendDeprecatedCode otlptrace.Status_DeprecatedStatusCode + expectedRcvCode otlptrace.Status_StatusCode + expectedDeprecatedCode otlptrace.Status_DeprecatedStatusCode }{ { // If code==STATUS_CODE_UNSET then the value of `deprecated_code` is the @@ -206,83 +207,93 @@ func TestDeprecatedStatusCode(t *testing.T) { // // if deprecated_code==DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret // the overall status to be STATUS_CODE_UNSET. - sendCode: otlptrace.Status_STATUS_CODE_UNSET, - sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, - expectedRcvCode: otlptrace.Status_STATUS_CODE_UNSET, + sendCode: otlptrace.Status_STATUS_CODE_UNSET, + sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, + expectedRcvCode: otlptrace.Status_STATUS_CODE_UNSET, + expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, }, { // if deprecated_code!=DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret // the overall status to be STATUS_CODE_ERROR. - sendCode: otlptrace.Status_STATUS_CODE_UNSET, - sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, - expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR, + sendCode: otlptrace.Status_STATUS_CODE_UNSET, + sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED, + expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR, + expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED, }, { // If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be - // ignored, the `code` field is the sole carrier of the status. - sendCode: otlptrace.Status_STATUS_CODE_OK, - sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, - expectedRcvCode: otlptrace.Status_STATUS_CODE_OK, + // overwritten, the `code` field is the sole carrier of the status. + sendCode: otlptrace.Status_STATUS_CODE_OK, + sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, + expectedRcvCode: otlptrace.Status_STATUS_CODE_OK, + expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, }, { // If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be - // ignored, the `code` field is the sole carrier of the status. - sendCode: otlptrace.Status_STATUS_CODE_OK, - sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, - expectedRcvCode: otlptrace.Status_STATUS_CODE_OK, + // overwritten, the `code` field is the sole carrier of the status. + sendCode: otlptrace.Status_STATUS_CODE_OK, + sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, + expectedRcvCode: otlptrace.Status_STATUS_CODE_OK, + expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, }, { // If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be - // ignored, the `code` field is the sole carrier of the status. - sendCode: otlptrace.Status_STATUS_CODE_ERROR, - sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, - expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR, + // overwritten, the `code` field is the sole carrier of the status. + sendCode: otlptrace.Status_STATUS_CODE_ERROR, + sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK, + expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR, + expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, }, { // If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be - // ignored, the `code` field is the sole carrier of the status. - sendCode: otlptrace.Status_STATUS_CODE_ERROR, - sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, - expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR, + // overwritten, the `code` field is the sole carrier of the status. + sendCode: otlptrace.Status_STATUS_CODE_ERROR, + sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, + expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR, + expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, }, } for _, test := range tests { - resourceSpans := []*otlptrace.ResourceSpans{ - { - InstrumentationLibrarySpans: []*otlptrace.InstrumentationLibrarySpans{ - { - Spans: []*otlptrace.Span{ - { - Status: otlptrace.Status{ - Code: test.sendCode, - DeprecatedCode: test.sendDeprecatedCode, + t.Run(test.sendCode.String()+"/"+test.sendDeprecatedCode.String(), func(t *testing.T) { + resourceSpans := []*otlptrace.ResourceSpans{ + { + InstrumentationLibrarySpans: []*otlptrace.InstrumentationLibrarySpans{ + { + Spans: []*otlptrace.Span{ + { + Status: otlptrace.Status{ + Code: test.sendCode, + DeprecatedCode: test.sendDeprecatedCode, + }, }, }, }, }, }, - }, - } + } - req := &collectortrace.ExportTraceServiceRequest{ - ResourceSpans: resourceSpans, - } + req := &collectortrace.ExportTraceServiceRequest{ + ResourceSpans: resourceSpans, + } + + traceSink.Reset() - traceSink.Reset() + resp, err := traceClient.Export(context.Background(), req) + require.NoError(t, err, "Failed to export trace: %v", err) + require.NotNil(t, resp, "The response is missing") - resp, err := traceClient.Export(context.Background(), req) - require.NoError(t, err, "Failed to export trace: %v", err) - require.NotNil(t, resp, "The response is missing") + require.Equal(t, 1, len(traceSink.AllTraces()), "unexpected length: %v", len(traceSink.AllTraces())) - require.Equal(t, 1, len(traceSink.AllTraces()), "unexpected length: %v", len(traceSink.AllTraces())) + rcvdStatus := traceSink.AllTraces()[0].ResourceSpans().At(0).InstrumentationLibrarySpans().At(0).Spans().At(0).Status() - rcvdStatus := traceSink.AllTraces()[0].ResourceSpans().At(0).InstrumentationLibrarySpans().At(0).Spans().At(0).Status() + // Check that Code is as expected. + assert.EqualValues(t, rcvdStatus.Code(), test.expectedRcvCode) - // Check that Code is as expected. - assert.EqualValues(t, rcvdStatus.Code(), test.expectedRcvCode) + spanProto := pdata.TracesToOtlp(traceSink.AllTraces()[0])[0].InstrumentationLibrarySpans[0].Spans[0] - // Check that DeprecatedCode is passed as is. - assert.EqualValues(t, rcvdStatus.DeprecatedCode(), test.sendDeprecatedCode) + // Check that DeprecatedCode is passed as is. + assert.EqualValues(t, spanProto.Status.DeprecatedCode, test.expectedDeprecatedCode) + }) } }