Skip to content

Commit 52164ac

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 ea9d10d commit 52164ac

File tree

7 files changed

+78
-124
lines changed

7 files changed

+78
-124
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 & 15 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: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -123,35 +123,6 @@ const (
123123
SpanKindCONSUMER = SpanKind(otlptrace.Span_SPAN_KIND_CONSUMER)
124124
)
125125

126-
// DeprecatedStatusCode is the deprecated status code used previously.
127-
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
128-
// Deprecated: use StatusCode instead.
129-
type DeprecatedStatusCode otlptrace.Status_DeprecatedStatusCode
130-
131-
const (
132-
DeprecatedStatusCodeOk = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_OK)
133-
DeprecatedStatusCodeCancelled = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_CANCELLED)
134-
DeprecatedStatusCodeUnknownError = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR)
135-
DeprecatedStatusCodeInvalidArgument = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_INVALID_ARGUMENT)
136-
DeprecatedStatusCodeDeadlineExceeded = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_DEADLINE_EXCEEDED)
137-
DeprecatedStatusCodeNotFound = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_NOT_FOUND)
138-
DeprecatedStatusCodeAlreadyExists = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_ALREADY_EXISTS)
139-
DeprecatedStatusCodePermissionDenied = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_PERMISSION_DENIED)
140-
DeprecatedStatusCodeResourceExhausted = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_RESOURCE_EXHAUSTED)
141-
DeprecatedStatusCodeFailedPrecondition = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_FAILED_PRECONDITION)
142-
DeprecatedStatusCodeAborted = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_ABORTED)
143-
DeprecatedStatusCodeOutOfRange = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_OUT_OF_RANGE)
144-
DeprecatedStatusCodeUnimplemented = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNIMPLEMENTED)
145-
DeprecatedStatusCodeInternalError = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_INTERNAL_ERROR)
146-
DeprecatedStatusCodeUnavailable = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNAVAILABLE)
147-
DeprecatedStatusCodeDataLoss = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_DATA_LOSS)
148-
DeprecatedStatusCodeUnauthenticated = DeprecatedStatusCode(otlptrace.Status_DEPRECATED_STATUS_CODE_UNAUTHENTICATED)
149-
)
150-
151-
func (sc DeprecatedStatusCode) String() string {
152-
return otlptrace.Status_DeprecatedStatusCode(sc).String()
153-
}
154-
155126
// StatusCode mirrors the codes defined at
156127
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
157128
type StatusCode otlptrace.Status_StatusCode
@@ -181,8 +152,8 @@ func (ms SpanStatus) SetCode(v StatusCode) {
181152
// set to DEPRECATED_STATUS_CODE_UNKNOWN_ERROR.
182153
switch v {
183154
case StatusCodeUnset, StatusCodeOk:
184-
ms.SetDeprecatedCode(DeprecatedStatusCodeOk)
155+
ms.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK
185156
case StatusCodeError:
186-
ms.SetDeprecatedCode(DeprecatedStatusCodeUnknownError)
157+
ms.orig.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR
187158
}
188159
}

‎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: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,16 @@ func (r *Receiver) Export(ctx context.Context, req *collectortrace.ExportTraceSe
7272
for _, rss := range req.ResourceSpans {
7373
for _, ils := range rss.InstrumentationLibrarySpans {
7474
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
75+
switch span.Status.Code {
76+
case otlptrace.Status_STATUS_CODE_UNSET:
77+
if span.Status.DeprecatedCode != otlptrace.Status_DEPRECATED_STATUS_CODE_OK {
78+
span.Status.Code = otlptrace.Status_STATUS_CODE_ERROR
79+
}
80+
case otlptrace.Status_STATUS_CODE_OK:
81+
// If status code is set then overwrites deprecated.
82+
span.Status.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_OK
83+
case otlptrace.Status_STATUS_CODE_ERROR:
84+
span.Status.DeprecatedCode = otlptrace.Status_DEPRECATED_STATUS_CODE_UNKNOWN_ERROR
7885
}
7986
}
8087
}

‎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)