Skip to content

Commit ceda5aa

Browse files
bogdandrutubogdandrutu
committed
Remove accessors for deprecated status code, fix receiver logic
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. This change is to be consistent with `SetCode`. Signed-off-by: Bogdan Drutu <[email protected]>
1 parent 9383e82 commit ceda5aa

File tree

7 files changed

+83
-145
lines changed

7 files changed

+83
-145
lines changed

‎cmd/pdatagen/internal/trace_structs.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,6 @@ var spanStatus = &messageValueStruct{
196196
// to OTLP spec https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231
197197
manualSetter: true,
198198
},
199-
&primitiveTypedField{
200-
fieldName: "DeprecatedCode",
201-
originFieldName: "DeprecatedCode",
202-
returnType: "DeprecatedStatusCode",
203-
rawType: "otlptrace.Status_DeprecatedStatusCode",
204-
defaultVal: "DeprecatedStatusCode(0)",
205-
testVal: "DeprecatedStatusCode(1)",
206-
},
207199
&primitiveField{
208200
fieldName: "Message",
209201
originFieldName: "Message",

‎consumer/pdata/generated_trace.go‎

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎consumer/pdata/generated_trace_test.go‎

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎consumer/pdata/trace.go‎

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -121,35 +121,6 @@ const (
121121
SpanKindCONSUMER = SpanKind(otlptrace.Span_SPAN_KIND_CONSUMER)
122122
)
123123

124-
// DeprecatedStatusCode is the deprecated status code used previously.
125-
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
126-
// Deprecated: use StatusCode instead.
127-
type DeprecatedStatusCode otlptrace.Status_DeprecatedStatusCode
128-
129-
const (
130-
DeprecatedStatusCodeOk = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_OK)
131-
DeprecatedStatusCodeCancelled = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_CANCELLED)
132-
DeprecatedStatusCodeUnknownError = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR)
133-
DeprecatedStatusCodeInvalidArgument = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_INVALID_ARGUMENT)
134-
DeprecatedStatusCodeDeadlineExceeded = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_DEADLINE_EXCEEDED)
135-
DeprecatedStatusCodeNotFound = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_NOT_FOUND)
136-
DeprecatedStatusCodeAlreadyExists = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_ALREADY_EXISTS)
137-
DeprecatedStatusCodePermissionDenied = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_PERMISSION_DENIED)
138-
DeprecatedStatusCodeResourceExhausted = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_RESOURCE_EXHAUSTED)
139-
DeprecatedStatusCodeFailedPrecondition = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_FAILED_PRECONDITION)
140-
DeprecatedStatusCodeAborted = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED)
141-
DeprecatedStatusCodeOutOfRange = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_OUT_OF_RANGE)
142-
DeprecatedStatusCodeUnimplemented = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNIMPLEMENTED)
143-
DeprecatedStatusCodeInternalError = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_INTERNAL_ERROR)
144-
DeprecatedStatusCodeUnavailable = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNAVAILABLE)
145-
DeprecatedStatusCodeDataLoss = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_DATA_LOSS)
146-
DeprecatedStatusCodeUnauthenticated = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNAUTHENTICATED)
147-
)
148-
149-
func (sc DeprecatedStatusCode) String() string {
150-
return otlptrace.Status_DeprecatedStatusCode(sc).String()
151-
}
152-
153124
// StatusCode mirrors the codes defined at
154125
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
155126
type StatusCode otlptrace.Status_StatusCode
@@ -166,21 +137,12 @@ func (sc StatusCode) String() string { return otlptrace.Status_StatusCode(sc).St
166137
func (ms SpanStatus) SetCode(v StatusCode) {
167138
ms.orig.Code = otlptrace.Status_StatusCode(v)
168139

169-
// According to OTLP spec we also need to set the deprecated_code field.
170-
// See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231
171-
//
172-
// if code==STATUS_CODE_UNSET then `deprecated_code` MUST be
173-
// set to DEPRECATED_STATUS_CODE_OK.
174-
//
175-
// if code==STATUS_CODE_OK then `deprecated_code` MUST be
176-
// set to DEPRECATED_STATUS_CODE_OK.
177-
//
178-
// if code==STATUS_CODE_ERROR then `deprecated_code` MUST be
179-
// set to DEPRECATED_STATUS_CODE_UNKNOWN_ERROR.
140+
// According to OTLP spec we also need to set the deprecated_code field as we are a new sender:
141+
// See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L239
180142
switch v {
181143
case StatusCodeUnset, StatusCodeOk:
182-
ms.SetDeprecatedCode(DeprecatedStatusCodeOk)
144+
ms.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK
183145
case StatusCodeError:
184-
ms.SetDeprecatedCode(DeprecatedStatusCodeUnknownError)
146+
ms.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR
185147
}
186148
}

‎consumer/pdata/trace_test.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,24 +128,21 @@ func TestSpanStatusCode(t *testing.T) {
128128
//
129129
// if code==STATUS_CODE_UNSET then `deprecated_code` MUST be
130130
// set to DEPRECATED_STATUS_CODE_OK.
131-
status.SetDeprecatedCode(DeprecatedStatusCodeUnknownError)
132-
assert.EqualValues(t, DeprecatedStatusCodeUnknownError, status.DeprecatedCode())
131+
status.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR
133132
status.SetCode(StatusCodeUnset)
134-
assert.EqualValues(t, DeprecatedStatusCodeOk, status.DeprecatedCode())
133+
assert.EqualValues(t, otlptrace.Status_DEPRECATED_STATUS_CODE_OK, status.orig.DeprecatedCode)
135134

136135
// if code==STATUS_CODE_OK then `deprecated_code` MUST be
137136
// set to DEPRECATED_STATUS_CODE_OK.
138-
status.SetDeprecatedCode(DeprecatedStatusCodeUnknownError)
139-
assert.EqualValues(t, DeprecatedStatusCodeUnknownError, status.DeprecatedCode())
137+
status.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR
140138
status.SetCode(StatusCodeOk)
141-
assert.EqualValues(t, DeprecatedStatusCodeOk, status.DeprecatedCode())
139+
assert.EqualValues(t, otlptrace.Status_DEPRECATED_STATUS_CODE_OK, status.orig.DeprecatedCode)
142140

143141
// if code==STATUS_CODE_ERROR then `deprecated_code` MUST be
144142
// set to DEPRECATED_STATUS_CODE_UNKNOWN_ERROR.
145-
status.SetDeprecatedCode(DeprecatedStatusCodeOk)
146-
assert.EqualValues(t, DeprecatedStatusCodeOk, status.DeprecatedCode())
143+
status.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK
147144
status.SetCode(StatusCodeError)
148-
assert.EqualValues(t, DeprecatedStatusCodeUnknownError, status.DeprecatedCode())
145+
assert.EqualValues(t, otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR, status.orig.DeprecatedCode)
149146
}
150147

151148
func TestToFromOtlp(t *testing.T) {

‎receiver/otlpreceiver/trace/otlp.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,26 +55,22 @@ func (r *Receiver) Export(ctx context.Context, req *collectortrace.ExportTraceSe
5555
ctxWithReceiverName := obsreport.ReceiverContext(ctx, r.instanceName, receiverTransport)
5656

5757
// Perform backward compatibility conversion of Span Status code according to
58-
// OTLP specification.
59-
// See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231
60-
//
61-
// If code==STATUS_CODE_UNSET then the value of `deprecated_code` is the
62-
// carrier of the overall status according to these rules:
63-
//
64-
// if deprecated_code==DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret
65-
// the overall status to be STATUS_CODE_UNSET.
66-
//
67-
// if deprecated_code!=DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret
68-
// the overall status to be STATUS_CODE_ERROR.
69-
//
70-
// If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be
71-
// ignored, the `code` field is the sole carrier of the status.
58+
// OTLP specification as we are a new receiver and sender (we are pushing data to the pipelines):
59+
// See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L239
60+
// See https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L253
7261
for _, rss := range req.ResourceSpans {
7362
for _, ils := range rss.InstrumentationLibrarySpans {
7463
for _, span := range ils.Spans {
75-
if span.Status.Code == otlptrace.Status_STATUS_CODE_UNSET &&
76-
span.Status.DeprecatedCode != otlptrace.Status_DEPRECATED_STATUS_CODE_OK {
77-
span.Status.Code = otlptrace.Status_STATUS_CODE_ERROR
64+
switch span.Status.Code {
65+
case otlptrace.Status_STATUS_CODE_UNSET:
66+
if span.Status.DeprecatedCode != otlptrace.Status_DEPRECATED_STATUS_CODE_OK {
67+
span.Status.Code = otlptrace.Status_STATUS_CODE_ERROR
68+
}
69+
case otlptrace.Status_STATUS_CODE_OK:
70+
// If status code is set then overwrites deprecated.
71+
span.Status.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK
72+
case otlptrace.Status_STATUS_CODE_ERROR:
73+
span.Status.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR
7874
}
7975
}
8076
}

‎receiver/otlpreceiver/trace/otlp_test.go‎

Lines changed: 60 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -196,93 +196,104 @@ func TestDeprecatedStatusCode(t *testing.T) {
196196
// See specification for handling status code here:
197197
// https://github.com/open-telemetry/opentelemetry-proto/blob/59c488bfb8fb6d0458ad6425758b70259ff4a2bd/opentelemetry/proto/trace/v1/trace.proto#L231
198198
tests := []struct {
199-
sendCode otlptrace.Status_StatusCode
200-
sendDeprecatedCode otlptrace.Status_DeprecatedStatusCode
201-
expectedRcvCode otlptrace.Status_StatusCode
199+
sendCode otlptrace.Status_StatusCode
200+
sendDeprecatedCode otlptrace.Status_DeprecatedStatusCode
201+
expectedRcvCode otlptrace.Status_StatusCode
202+
expectedDeprecatedCode otlptrace.Status_DeprecatedStatusCode
202203
}{
203204
{
204205
// If code==STATUS_CODE_UNSET then the value of `deprecated_code` is the
205206
// carrier of the overall status according to these rules:
206207
//
207208
// if deprecated_code==DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret
208209
// the overall status to be STATUS_CODE_UNSET.
209-
sendCode: otlptrace.Status_STATUS_CODE_UNSET,
210-
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
211-
expectedRcvCode: otlptrace.Status_STATUS_CODE_UNSET,
210+
sendCode: otlptrace.Status_STATUS_CODE_UNSET,
211+
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
212+
expectedRcvCode: otlptrace.Status_STATUS_CODE_UNSET,
213+
expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
212214
},
213215
{
214216
// if deprecated_code!=DEPRECATED_STATUS_CODE_OK then the receiver MUST interpret
215217
// the overall status to be STATUS_CODE_ERROR.
216-
sendCode: otlptrace.Status_STATUS_CODE_UNSET,
217-
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
218-
expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR,
218+
sendCode: otlptrace.Status_STATUS_CODE_UNSET,
219+
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED,
220+
expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR,
221+
expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED,
219222
},
220223
{
221224
// If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be
222-
// ignored, the `code` field is the sole carrier of the status.
223-
sendCode: otlptrace.Status_STATUS_CODE_OK,
224-
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
225-
expectedRcvCode: otlptrace.Status_STATUS_CODE_OK,
225+
// overwritten, the `code` field is the sole carrier of the status.
226+
sendCode: otlptrace.Status_STATUS_CODE_OK,
227+
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
228+
expectedRcvCode: otlptrace.Status_STATUS_CODE_OK,
229+
expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
226230
},
227231
{
228232
// If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be
229-
// ignored, the `code` field is the sole carrier of the status.
230-
sendCode: otlptrace.Status_STATUS_CODE_OK,
231-
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
232-
expectedRcvCode: otlptrace.Status_STATUS_CODE_OK,
233+
// overwritten, the `code` field is the sole carrier of the status.
234+
sendCode: otlptrace.Status_STATUS_CODE_OK,
235+
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
236+
expectedRcvCode: otlptrace.Status_STATUS_CODE_OK,
237+
expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
233238
},
234239
{
235240
// If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be
236-
// ignored, the `code` field is the sole carrier of the status.
237-
sendCode: otlptrace.Status_STATUS_CODE_ERROR,
238-
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
239-
expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR,
241+
// overwritten, the `code` field is the sole carrier of the status.
242+
sendCode: otlptrace.Status_STATUS_CODE_ERROR,
243+
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_OK,
244+
expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR,
245+
expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
240246
},
241247
{
242248
// If code!=STATUS_CODE_UNSET then the value of `deprecated_code` MUST be
243-
// ignored, the `code` field is the sole carrier of the status.
244-
sendCode: otlptrace.Status_STATUS_CODE_ERROR,
245-
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
246-
expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR,
249+
// overwritten, the `code` field is the sole carrier of the status.
250+
sendCode: otlptrace.Status_STATUS_CODE_ERROR,
251+
sendDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
252+
expectedRcvCode: otlptrace.Status_STATUS_CODE_ERROR,
253+
expectedDeprecatedCode: otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR,
247254
},
248255
}
249256

250257
for _, test := range tests {
251-
resourceSpans := []*otlptrace.ResourceSpans{
252-
{
253-
InstrumentationLibrarySpans: []*otlptrace.InstrumentationLibrarySpans{
254-
{
255-
Spans: []*otlptrace.Span{
256-
{
257-
Status: otlptrace.Status{
258-
Code: test.sendCode,
259-
DeprecatedCode: test.sendDeprecatedCode,
258+
t.Run(test.sendCode.String()+"/"+test.sendDeprecatedCode.String(), func(t *testing.T) {
259+
resourceSpans := []*otlptrace.ResourceSpans{
260+
{
261+
InstrumentationLibrarySpans: []*otlptrace.InstrumentationLibrarySpans{
262+
{
263+
Spans: []*otlptrace.Span{
264+
{
265+
Status: otlptrace.Status{
266+
Code: test.sendCode,
267+
DeprecatedCode: test.sendDeprecatedCode,
268+
},
260269
},
261270
},
262271
},
263272
},
264273
},
265-
},
266-
}
274+
}
267275

268-
req := &collectortrace.ExportTraceServiceRequest{
269-
ResourceSpans: resourceSpans,
270-
}
276+
req := &collectortrace.ExportTraceServiceRequest{
277+
ResourceSpans: resourceSpans,
278+
}
279+
280+
traceSink.Reset()
271281

272-
traceSink.Reset()
282+
resp, err := traceClient.Export(context.Background(), req)
283+
require.NoError(t, err, "Failed to export trace: %v", err)
284+
require.NotNil(t, resp, "The response is missing")
273285

274-
resp, err := traceClient.Export(context.Background(), req)
275-
require.NoError(t, err, "Failed to export trace: %v", err)
276-
require.NotNil(t, resp, "The response is missing")
286+
require.Equal(t, 1, len(traceSink.AllTraces()), "unexpected length: %v", len(traceSink.AllTraces()))
277287

278-
require.Equal(t, 1, len(traceSink.AllTraces()), "unexpected length: %v", len(traceSink.AllTraces()))
288+
rcvdStatus := traceSink.AllTraces()[0].ResourceSpans().At(0).InstrumentationLibrarySpans().At(0).Spans().At(0).Status()
279289

280-
rcvdStatus := traceSink.AllTraces()[0].ResourceSpans().At(0).InstrumentationLibrarySpans().At(0).Spans().At(0).Status()
290+
// Check that Code is as expected.
291+
assert.EqualValues(t, rcvdStatus.Code(), test.expectedRcvCode)
281292

282-
// Check that Code is as expected.
283-
assert.EqualValues(t, rcvdStatus.Code(), test.expectedRcvCode)
293+
spanProto := pdata.TracesToOtlp(traceSink.AllTraces()[0])[0].InstrumentationLibrarySpans[0].Spans[0]
284294

285-
// Check that DeprecatedCode is passed as is.
286-
assert.EqualValues(t, rcvdStatus.DeprecatedCode(), test.sendDeprecatedCode)
295+
// Check that DeprecatedCode is passed as is.
296+
assert.EqualValues(t, spanProto.Status.DeprecatedCode, test.expectedDeprecatedCode)
297+
})
287298
}
288299
}

0 commit comments

Comments
 (0)