Skip to content

Commit

Permalink
Remove accessors for deprecated status code, fix receiver logic
Browse files Browse the repository at this point in the history 
The previous logic was to ignore deprecated if received non unset for new status code,
but if downstream a service is not upgraded it should see the deprecated status set correctly.

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
@bogdandrutu
bogdandrutu committed Feb 20, 2021
1 parent ea9d10d commit c7664b6
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 124 deletions.
8 changes: 0 additions & 8 deletions cmd/pdatagen/internal/trace_structs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 0 additions & 15 deletions consumer/pdata/generated_trace.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 0 additions & 9 deletions consumer/pdata/generated_trace_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 2 additions & 31 deletions consumer/pdata/trace.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,35 +123,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
Expand Down Expand Up @@ -181,8 +152,8 @@ func (ms SpanStatus) SetCode(v StatusCode) {
// set to DEPRECATED_STATUS_CODE_UNKNOWN_ERROR.
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
}
}
15 changes: 6 additions & 9 deletions consumer/pdata/trace_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
13 changes: 10 additions & 3 deletions receiver/otlpreceiver/trace/otlp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,16 @@ func (r *Receiver) Export(ctx context.Context, req *collectortrace.ExportTraceSe
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
}
}
}
Expand Down
109 changes: 60 additions & 49 deletions receiver/otlpreceiver/trace/otlp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,93 +196,104 @@ 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
// 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.
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)
})
}
}

0 comments on commit c7664b6

Please sign in to comment.