From 20f11998c6ece9b8e69b1e1acaaa173277727630 Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Mon, 5 May 2025 12:56:37 -0400 Subject: [PATCH 1/9] [trace] add Unmarshaler functionality to SpanContext and subfields --- trace/trace.go | 91 ++++++++++++++- trace/trace_test.go | 234 +++++++++++++++++++++++++++++++++++++++ trace/tracestate.go | 23 ++++ trace/tracestate_test.go | 73 ++++++++++++ 4 files changed, 415 insertions(+), 6 deletions(-) diff --git a/trace/trace.go b/trace/trace.go index d49adf671b9..ccf8f59db01 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -35,7 +35,18 @@ type TraceID [16]byte var ( nilTraceID TraceID - _ json.Marshaler = nilTraceID + _ json.Marshaler = nilTraceID + _ json.Unmarshaler = &nilTraceID + + nilSpanID SpanID + _ json.Marshaler = nilSpanID + _ json.Unmarshaler = &nilSpanID + + nilTraceFlags TraceFlags + _ json.Marshaler = nilTraceFlags + _ json.Unmarshaler = &nilTraceFlags + + _ json.Unmarshaler = &SpanContext{} ) // IsValid checks whether the trace TraceID is valid. A valid trace ID does @@ -55,14 +66,30 @@ func (t TraceID) String() string { return hex.EncodeToString(t[:]) } +// UnmarshalJSON implements the json.Unmarshaler interface for TraceID. +// It expects the JSON to be a string (as produced by MarshalJSON), parses +// it via TraceIDFromHex, and replaces the receiver's contents. +func (t *TraceID) UnmarshalJSON(data []byte) error { + // 1) Unmarshal the JSON payload into a Go string. + var raw string + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + // 2) Parse that string into a TraceID. + parsed, err := TraceIDFromHex(raw) + if err != nil { + return err + } + + // 3) Overwrite the receiver with the parsed result. + *t = parsed + return nil +} + // SpanID is a unique identity of a span in a trace. type SpanID [8]byte -var ( - nilSpanID SpanID - _ json.Marshaler = nilSpanID -) - // IsValid checks whether the SpanID is valid. A valid SpanID does not consist // of zeros only. func (s SpanID) IsValid() bool { @@ -80,6 +107,27 @@ func (s SpanID) String() string { return hex.EncodeToString(s[:]) } +// UnmarshalJSON implements the json.Unmarshaler interface for SpanID. +// It expects the JSON to be a string (as produced by MarshalJSON), parses +// it via SpanIDFromHex, and replaces the receiver's contents. +func (s *SpanID) UnmarshalJSON(data []byte) error { + // 1) Unmarshal the JSON payload into a Go string. + var raw string + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + // 2) Parse that string into a SpanID. + parsed, err := SpanIDFromHex(raw) + if err != nil { + return err + } + + // 3) Overwrite the receiver with the parsed result. + *s = parsed + return nil +} + // TraceIDFromHex returns a TraceID from a hex string if it is compliant with // the W3C trace-context specification. See more at // https://www.w3.org/TR/trace-context/#trace-id @@ -168,6 +216,25 @@ func (tf TraceFlags) String() string { return hex.EncodeToString([]byte{byte(tf)}[:]) } +// UnmarshalJSON implements the json.Unmarshaler interface for TraceFlags. +// It expects the JSON to be a hex string (as produced by MarshalJSON), parses +// it and sets the value. +func (tf *TraceFlags) UnmarshalJSON(data []byte) error { + var raw string + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + decoded, err := hex.DecodeString(raw) + if err != nil { + return err + } + if len(decoded) != 1 { + return errInvalidTraceIDLength // Use a relevant error or define a new one for TraceFlags + } + *tf = TraceFlags(decoded[0]) + return nil +} + // SpanContextConfig contains mutable fields usable for constructing // an immutable SpanContext. type SpanContextConfig struct { @@ -321,3 +388,15 @@ func (sc SpanContext) MarshalJSON() ([]byte, error) { Remote: sc.remote, }) } + +// UnmarshalJSON implements the json.Unmarshaler interface for SpanContext. +// It expects the JSON to be an object with fields matching SpanContextConfig, +// and uses NewSpanContext to construct the immutable SpanContext value. +func (sc *SpanContext) UnmarshalJSON(data []byte) error { + var cfg SpanContextConfig + if err := json.Unmarshal(data, &cfg); err != nil { + return err + } + *sc = NewSpanContext(cfg) + return nil +} diff --git a/trace/trace_test.go b/trace/trace_test.go index 9abea43a036..99353a42eac 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -5,6 +5,7 @@ package trace import ( "bytes" + "encoding/json" "testing" "github.com/google/go-cmp/cmp" @@ -571,3 +572,236 @@ func TestConfigLinkMutability(t *testing.T) { want := SpanConfig{links: []Link{l0, l1}} assert.Equal(t, want, conf) } + +func TestTraceIDUnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want TraceID + wantErr bool + }{ + { + name: "valid TraceID", + input: `"80f198ee56343ba864fe8b2a57d3eff7"`, + want: TraceID{0x80, 0xf1, 0x98, 0xee, 0x56, 0x34, 0x3b, 0xa8, 0x64, 0xfe, 0x8b, 0x2a, 0x57, 0xd3, 0xef, 0xf7}, + }, + { + name: "invalid length", + input: `"80f198ee56343ba864fe8b2a57d3eff"`, + wantErr: true, + }, + { + name: "invalid char", + input: `"80f198ee56343ba864fe8b2a57d3efg7"`, + wantErr: true, + }, + { + name: "all zeros", + input: `"00000000000000000000000000000000"`, + wantErr: true, + }, + { + name: "not a string", + input: `123`, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var tid TraceID + err := json.Unmarshal([]byte(tc.input), &tid) + if tc.wantErr { + if err == nil { + t.Errorf("expected error, got nil") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if tid != tc.want { + t.Errorf("got %v, want %v", tid, tc.want) + } + } + }) + } +} + +func TestSpanIDUnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want SpanID + wantErr bool + }{ + { + name: "valid SpanID", + input: `"2a00000000000000"`, + want: SpanID{0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}, + }, + { + name: "invalid length", + input: `"2a0000000000000"`, + wantErr: true, + }, + { + name: "invalid char", + input: `"2a0000000000000g"`, + wantErr: true, + }, + { + name: "all zeros", + input: `"0000000000000000"`, + wantErr: true, + }, + { + name: "not a string", + input: `123`, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var sid SpanID + err := json.Unmarshal([]byte(tc.input), &sid) + if tc.wantErr { + if err == nil { + t.Errorf("expected error, got nil") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if sid != tc.want { + t.Errorf("got %v, want %v", sid, tc.want) + } + } + }) + } +} + +func TestTraceFlagsUnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want TraceFlags + wantErr bool + }{ + { + name: "valid TraceFlags 01", + input: `"01"`, + want: TraceFlags(0x01), + }, + { + name: "valid TraceFlags 00", + input: `"00"`, + want: TraceFlags(0x00), + }, + { + name: "invalid hex", + input: `"gg"`, + wantErr: true, + }, + { + name: "invalid length", + input: `"0102"`, + wantErr: true, + }, + { + name: "not a string", + input: `123`, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var tf TraceFlags + err := json.Unmarshal([]byte(tc.input), &tf) + if tc.wantErr { + if err == nil { + t.Errorf("expected error, got nil") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if tf != tc.want { + t.Errorf("got %v, want %v", tf, tc.want) + } + } + }) + } +} + +func TestSpanContextUnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want SpanContext + wantErr bool + }{ + { + name: "valid full SpanContext", + input: `{"TraceID":"01000000000000000000000000000000","SpanID":"2a00000000000000","TraceFlags":"01","TraceState":"foo=1","Remote":true}`, + want: NewSpanContext(SpanContextConfig{ + TraceID: TraceID{0x01}, + SpanID: SpanID{0x2a}, + TraceFlags: TraceFlags(0x01), + TraceState: TraceState{list: []member{{Key: "foo", Value: "1"}}}, + Remote: true, + }), + }, + { + name: "valid partial SpanContext", + input: `{"TraceID":"01000000000000000000000000000000","SpanID":"2a00000000000000"}`, + want: NewSpanContext(SpanContextConfig{ + TraceID: TraceID{0x01}, + SpanID: SpanID{0x2a}, + }), + }, + { + name: "invalid TraceID", + input: `{"TraceID":"00000000000000000000000000000000","SpanID":"2a00000000000000"}`, + wantErr: true, + }, + { + name: "invalid JSON", + input: `{"TraceID":123}`, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var sc SpanContext + err := json.Unmarshal([]byte(tc.input), &sc) + if tc.wantErr { + if err == nil { + t.Errorf("expected error, got nil") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if !assertSpanContextEqual(sc, tc.want) { + t.Errorf("got %+v, want %+v", sc, tc.want) + } + // Round-trip test + data, err := json.Marshal(sc) + if err != nil { + t.Errorf("MarshalJSON failed: %v", err) + } + var sc2 SpanContext + err = json.Unmarshal(data, &sc2) + if err != nil { + t.Errorf("UnmarshalJSON round-trip failed: %v", err) + } + if !assertSpanContextEqual(sc, sc2) { + t.Errorf("round-trip: got %+v, want %+v", sc2, sc) + } + } + }) + } +} diff --git a/trace/tracestate.go b/trace/tracestate.go index dc5e34cad0d..935919f2e85 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -174,6 +174,8 @@ type TraceState struct { //nolint:revive // revive complains about stutter of `t var _ json.Marshaler = TraceState{} +var _ json.Unmarshaler = &TraceState{} + // ParseTraceState attempts to decode a TraceState from the passed // string. It returns an error if the input is invalid according to the W3C // Trace Context specification. @@ -219,6 +221,27 @@ func (ts TraceState) MarshalJSON() ([]byte, error) { return json.Marshal(ts.String()) } +// UnmarshalJSON implements the json.Unmarshaler interface for TraceState. +// It expects the JSON to be a string (as produced by MarshalJSON), parses +// it via ParseTraceState, and replaces the receiver’s contents. +func (ts *TraceState) UnmarshalJSON(data []byte) error { + // 1) Unmarshal the JSON payload into a Go string. + var raw string + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + // 2) Parse that string into a TraceState. + parsed, err := ParseTraceState(raw) + if err != nil { + return err + } + + // 3) Overwrite the receiver with the parsed result. + *ts = parsed + return nil +} + // String encodes the TraceState into a string compliant with the W3C // Trace Context specification. The returned string will be invalid if the // TraceState contains any invalid members. diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index aff07d46c6f..2562abc606f 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -624,3 +624,76 @@ func BenchmarkParseTraceState(b *testing.B) { }) } } + +func TestTraceStateUnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want TraceState + wantErr bool + }{ + { + name: "valid tracestate", + input: `"foo=1,bar=2"`, + want: TraceState{list: []member{{Key: "foo", Value: "1"}, {Key: "bar", Value: "2"}}}, + }, + { + name: "duplicate key", + input: `"foo=1,foo=2"`, + wantErr: true, + }, + { + name: "invalid key", + input: `"foo!=1"`, + wantErr: true, + }, + { + name: "too many members", + input: `"` + generateTooManyMembersString() + `"`, + wantErr: true, + }, + { + name: "not a string", + input: `123`, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ts TraceState + err := json.Unmarshal([]byte(tc.input), &ts) + if tc.wantErr { + if err == nil { + t.Errorf("expected error, got nil") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if fmt.Sprint(ts) != fmt.Sprint(tc.want) { + t.Errorf("got %v, want %v", ts, tc.want) + } + } + }) + } +} + +func generateTooManyMembersString() string { + members := make([]string, 33) + for i := 0; i < 33; i++ { + members[i] = fmt.Sprintf("k%d=v%d", i, i) + } + return fmt.Sprintf("%s", stringJoin(members, ",")) +} + +func stringJoin(elems []string, sep string) string { + if len(elems) == 0 { + return "" + } + result := elems[0] + for _, s := range elems[1:] { + result += sep + s + } + return result +} From 7132e588c95509f63d862cc98a8bc0e113fa0865 Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Mon, 5 May 2025 13:06:34 -0400 Subject: [PATCH 2/9] make precommit --- trace/trace_test.go | 19 ++++++++++++++++++- trace/tracestate_test.go | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/trace/trace_test.go b/trace/trace_test.go index 99353a42eac..ad1b6c712c6 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -583,7 +583,24 @@ func TestTraceIDUnmarshalJSON(t *testing.T) { { name: "valid TraceID", input: `"80f198ee56343ba864fe8b2a57d3eff7"`, - want: TraceID{0x80, 0xf1, 0x98, 0xee, 0x56, 0x34, 0x3b, 0xa8, 0x64, 0xfe, 0x8b, 0x2a, 0x57, 0xd3, 0xef, 0xf7}, + want: TraceID{ + 0x80, + 0xf1, + 0x98, + 0xee, + 0x56, + 0x34, + 0x3b, + 0xa8, + 0x64, + 0xfe, + 0x8b, + 0x2a, + 0x57, + 0xd3, + 0xef, + 0xf7, + }, }, { name: "invalid length", diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index 2562abc606f..4a066201916 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -684,7 +684,7 @@ func generateTooManyMembersString() string { for i := 0; i < 33; i++ { members[i] = fmt.Sprintf("k%d=v%d", i, i) } - return fmt.Sprintf("%s", stringJoin(members, ",")) + return stringJoin(members, ",") } func stringJoin(elems []string, sep string) string { From 143e83af1f98c4fbafdc7ece58e5f7ae2230a147 Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Mon, 5 May 2025 13:12:51 -0400 Subject: [PATCH 3/9] add explicit Marshaler reference for SpanContext --- trace/trace.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/trace/trace.go b/trace/trace.go index ccf8f59db01..0f201ffef52 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -46,7 +46,9 @@ var ( _ json.Marshaler = nilTraceFlags _ json.Unmarshaler = &nilTraceFlags - _ json.Unmarshaler = &SpanContext{} + nilSpanContext SpanContext + _ json.Marshaler = nilSpanContext + _ json.Unmarshaler = &nilSpanContext ) // IsValid checks whether the trace TraceID is valid. A valid trace ID does From 011e215f4ee4669dc5726c532b81f18463753a7d Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Mon, 5 May 2025 13:15:42 -0400 Subject: [PATCH 4/9] add chloggen --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 687e493e21c..1d1d2914e9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm The package contains semantic conventions from the `v1.31.0` version of the OpenTelemetry Semantic Conventions. See the [migration documentation](./semconv/v1.31.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.30.0`(#6479) - Add `Recording`, `Scope`, and `Record` types in `go.opentelemetry.io/otel/log/logtest`. (#6507) +- Add explicit JSON Unmarshaler interface methods for `trace.SpanContext` objects and subfields in `go.opentelemetry.io/otel/trace` (#6738). ### Removed From acf0cf53353b0f8647e87de25eb2bb5511b341eb Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Tue, 6 May 2025 08:55:46 -0400 Subject: [PATCH 5/9] suggestions from code review OTEL-2540 --- trace/trace.go | 30 +++++++++++++++++------------- trace/trace_test.go | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/trace/trace.go b/trace/trace.go index 0f201ffef52..f24087b13c3 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -37,18 +37,6 @@ var ( nilTraceID TraceID _ json.Marshaler = nilTraceID _ json.Unmarshaler = &nilTraceID - - nilSpanID SpanID - _ json.Marshaler = nilSpanID - _ json.Unmarshaler = &nilSpanID - - nilTraceFlags TraceFlags - _ json.Marshaler = nilTraceFlags - _ json.Unmarshaler = &nilTraceFlags - - nilSpanContext SpanContext - _ json.Marshaler = nilSpanContext - _ json.Unmarshaler = &nilSpanContext ) // IsValid checks whether the trace TraceID is valid. A valid trace ID does @@ -92,6 +80,12 @@ func (t *TraceID) UnmarshalJSON(data []byte) error { // SpanID is a unique identity of a span in a trace. type SpanID [8]byte +var ( + nilSpanID SpanID + _ json.Marshaler = nilSpanID + _ json.Unmarshaler = &nilSpanID +) + // IsValid checks whether the SpanID is valid. A valid SpanID does not consist // of zeros only. func (s SpanID) IsValid() bool { @@ -193,6 +187,12 @@ func decodeHex(h string, b []byte) error { // TraceFlags contains flags that can be set on a SpanContext. type TraceFlags byte //nolint:revive // revive complains about stutter of `trace.TraceFlags`. +var ( + nilTraceFlags TraceFlags + _ json.Marshaler = nilTraceFlags + _ json.Unmarshaler = &nilTraceFlags +) + // IsSampled returns if the sampling bit is set in the TraceFlags. func (tf TraceFlags) IsSampled() bool { return tf&FlagsSampled == FlagsSampled @@ -268,7 +268,11 @@ type SpanContext struct { remote bool } -var _ json.Marshaler = SpanContext{} +var ( + nilSpanContext SpanContext + _ json.Marshaler = nilSpanContext + _ json.Unmarshaler = &nilSpanContext +) // IsValid returns if the SpanContext is valid. A valid span context has a // valid TraceID and SpanID. diff --git a/trace/trace_test.go b/trace/trace_test.go index ad1b6c712c6..52124b583a8 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -785,7 +785,7 @@ func TestSpanContextUnmarshalJSON(t *testing.T) { }, { name: "invalid JSON", - input: `{"TraceID":123}`, + input: `{"TraceID:123}`, wantErr: true, }, } From da90d7371fbd87b2955b7976f6c55f49d3e19ac6 Mon Sep 17 00:00:00 2001 From: "John L. Peterson (Jack)" Date: Tue, 6 May 2025 09:51:55 -0400 Subject: [PATCH 6/9] Update CHANGELOG.md Co-authored-by: Damien Mathieu <42@dmathieu.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d1d2914e9c..f74831e6d58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm The package contains semantic conventions from the `v1.31.0` version of the OpenTelemetry Semantic Conventions. See the [migration documentation](./semconv/v1.31.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.30.0`(#6479) - Add `Recording`, `Scope`, and `Record` types in `go.opentelemetry.io/otel/log/logtest`. (#6507) -- Add explicit JSON Unmarshaler interface methods for `trace.SpanContext` objects and subfields in `go.opentelemetry.io/otel/trace` (#6738). +- Add explicit JSON Unmarshaler interface methods for `SpanContext`, `TraceID`, `SpanID`, and `TraceFlags` in `go.opentelemetry.io/otel/trace`. (#6738) ### Removed From 008f3382656f74b73bc4da6938f360f1d8b0960b Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Tue, 6 May 2025 10:18:19 -0400 Subject: [PATCH 7/9] [OTEL-2540] remove unnecessary comments --- trace/trace.go | 20 +++++--------------- trace/tracestate.go | 6 +----- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/trace/trace.go b/trace/trace.go index f24087b13c3..2a63c30e852 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -57,22 +57,18 @@ func (t TraceID) String() string { } // UnmarshalJSON implements the json.Unmarshaler interface for TraceID. -// It expects the JSON to be a string (as produced by MarshalJSON), parses -// it via TraceIDFromHex, and replaces the receiver's contents. +// It expects the JSON to be a string (as produced by MarshalJSON). func (t *TraceID) UnmarshalJSON(data []byte) error { - // 1) Unmarshal the JSON payload into a Go string. var raw string if err := json.Unmarshal(data, &raw); err != nil { return err } - // 2) Parse that string into a TraceID. parsed, err := TraceIDFromHex(raw) if err != nil { return err } - // 3) Overwrite the receiver with the parsed result. *t = parsed return nil } @@ -104,22 +100,18 @@ func (s SpanID) String() string { } // UnmarshalJSON implements the json.Unmarshaler interface for SpanID. -// It expects the JSON to be a string (as produced by MarshalJSON), parses -// it via SpanIDFromHex, and replaces the receiver's contents. +// It expects the JSON to be a string (as produced by MarshalJSON). func (s *SpanID) UnmarshalJSON(data []byte) error { - // 1) Unmarshal the JSON payload into a Go string. var raw string if err := json.Unmarshal(data, &raw); err != nil { return err } - // 2) Parse that string into a SpanID. parsed, err := SpanIDFromHex(raw) if err != nil { return err } - // 3) Overwrite the receiver with the parsed result. *s = parsed return nil } @@ -219,8 +211,7 @@ func (tf TraceFlags) String() string { } // UnmarshalJSON implements the json.Unmarshaler interface for TraceFlags. -// It expects the JSON to be a hex string (as produced by MarshalJSON), parses -// it and sets the value. +// It expects the JSON to be a hex string (as produced by MarshalJSON). func (tf *TraceFlags) UnmarshalJSON(data []byte) error { var raw string if err := json.Unmarshal(data, &raw); err != nil { @@ -231,7 +222,7 @@ func (tf *TraceFlags) UnmarshalJSON(data []byte) error { return err } if len(decoded) != 1 { - return errInvalidTraceIDLength // Use a relevant error or define a new one for TraceFlags + return errInvalidTraceIDLength } *tf = TraceFlags(decoded[0]) return nil @@ -396,8 +387,7 @@ func (sc SpanContext) MarshalJSON() ([]byte, error) { } // UnmarshalJSON implements the json.Unmarshaler interface for SpanContext. -// It expects the JSON to be an object with fields matching SpanContextConfig, -// and uses NewSpanContext to construct the immutable SpanContext value. +// It expects the JSON to be an object with fields matching SpanContextConfig. func (sc *SpanContext) UnmarshalJSON(data []byte) error { var cfg SpanContextConfig if err := json.Unmarshal(data, &cfg); err != nil { diff --git a/trace/tracestate.go b/trace/tracestate.go index 935919f2e85..5410714c1de 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -222,22 +222,18 @@ func (ts TraceState) MarshalJSON() ([]byte, error) { } // UnmarshalJSON implements the json.Unmarshaler interface for TraceState. -// It expects the JSON to be a string (as produced by MarshalJSON), parses -// it via ParseTraceState, and replaces the receiver’s contents. +// It expects the JSON to be a string (as produced by MarshalJSON). func (ts *TraceState) UnmarshalJSON(data []byte) error { - // 1) Unmarshal the JSON payload into a Go string. var raw string if err := json.Unmarshal(data, &raw); err != nil { return err } - // 2) Parse that string into a TraceState. parsed, err := ParseTraceState(raw) if err != nil { return err } - // 3) Overwrite the receiver with the parsed result. *ts = parsed return nil } From e2435ee545d3aa4e688e8cd7c827305f63c5f922 Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Tue, 6 May 2025 10:20:53 -0400 Subject: [PATCH 8/9] [OTEL-2540] update SpanContext JSON tags to match OTel API Specification --- CHANGELOG.md | 1 + exporters/stdout/stdouttrace/trace_test.go | 12 ++++++------ trace/trace.go | 12 +++++++----- trace/trace_test.go | 12 ++++++------ 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f74831e6d58..84a540a167c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `go.opentelemetry.io/otel/sdk/log/logtest` is now a separate Go module. (#6466) - `Recorder` in `go.opentelemetry.io/otel/log/logtest` no longer separately stores records emitted by loggers with the same instrumentation scope. (#6507) - Improve performance of `BatchProcessor` in `go.opentelemetry.io/otel/sdk/log` by not exporting when exporter cannot accept more. (#6569, #6641) +- Update JSON tags for `TraceId`, `SpanId`, and `IsRemote` in `go.opentelemetry.io/otel/trace` to match [OpenTelemetry API specification for SpanContext items](https://github.com/open-telemetry/opentelemetry-specification/blob/815598814f3cf461ad5493ccbddd53633fb5cf24/specification/trace/api.md#spancontext) (#6738) ### Deprecated diff --git a/exporters/stdout/stdouttrace/trace_test.go b/exporters/stdout/stdouttrace/trace_test.go index a034eb40187..2a291d622e7 100644 --- a/exporters/stdout/stdouttrace/trace_test.go +++ b/exporters/stdout/stdouttrace/trace_test.go @@ -106,18 +106,18 @@ func expectedJSON(now time.Time) string { return `{ "Name": "/foo", "SpanContext": { - "TraceID": "0102030405060708090a0b0c0d0e0f10", - "SpanID": "0102030405060708", + "TraceId": "0102030405060708090a0b0c0d0e0f10", + "SpanId": "0102030405060708", "TraceFlags": "00", "TraceState": "key=val", - "Remote": false + "IsRemote": false }, "Parent": { - "TraceID": "00000000000000000000000000000000", - "SpanID": "0000000000000000", + "TraceId": "00000000000000000000000000000000", + "SpanId": "0000000000000000", "TraceFlags": "00", "TraceState": "", - "Remote": false + "IsRemote": false }, "SpanKind": 1, "StartTime": ` + string(serializedNow) + `, diff --git a/trace/trace.go b/trace/trace.go index 2a63c30e852..8e09fe88758 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -230,12 +230,14 @@ func (tf *TraceFlags) UnmarshalJSON(data []byte) error { // SpanContextConfig contains mutable fields usable for constructing // an immutable SpanContext. +// JSON tags match OTel API Specification: +// https://github.com/open-telemetry/opentelemetry-specification/blob/815598814f3cf461ad5493ccbddd53633fb5cf24/specification/trace/api.md#spancontext type SpanContextConfig struct { - TraceID TraceID - SpanID SpanID - TraceFlags TraceFlags - TraceState TraceState - Remote bool + TraceID TraceID `json:"TraceId"` + SpanID SpanID `json:"SpanId"` + TraceFlags TraceFlags `json:"TraceFlags"` + TraceState TraceState `json:"TraceState"` + Remote bool `json:"IsRemote"` } // NewSpanContext constructs a SpanContext using values from the provided diff --git a/trace/trace_test.go b/trace/trace_test.go index 52124b583a8..263a4746751 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -151,7 +151,7 @@ func TestSpanContextMarshalJSON(t *testing.T) { name: "SpanContext.MarshalJSON() returns json with partial data", tid: [16]byte{1}, sid: [8]byte{42}, - want: []byte(`{"TraceID":"01000000000000000000000000000000","SpanID":"2a00000000000000","TraceFlags":"00","TraceState":"","Remote":false}`), + want: []byte(`{"TraceId":"01000000000000000000000000000000","SpanId":"2a00000000000000","TraceFlags":"00","TraceState":"","IsRemote":false}`), }, { name: "SpanContext.MarshalJSON() returns json with full data", @@ -162,7 +162,7 @@ func TestSpanContextMarshalJSON(t *testing.T) { tstate: TraceState{list: []member{ {Key: "foo", Value: "1"}, }}, - want: []byte(`{"TraceID":"01000000000000000000000000000000","SpanID":"2a00000000000000","TraceFlags":"01","TraceState":"foo=1","Remote":true}`), + want: []byte(`{"TraceId":"01000000000000000000000000000000","SpanId":"2a00000000000000","TraceFlags":"01","TraceState":"foo=1","IsRemote":true}`), }, } { t.Run(testcase.name, func(t *testing.T) { @@ -761,7 +761,7 @@ func TestSpanContextUnmarshalJSON(t *testing.T) { }{ { name: "valid full SpanContext", - input: `{"TraceID":"01000000000000000000000000000000","SpanID":"2a00000000000000","TraceFlags":"01","TraceState":"foo=1","Remote":true}`, + input: `{"TraceId":"01000000000000000000000000000000","SpanId":"2a00000000000000","TraceFlags":"01","TraceState":"foo=1","IsRemote":true}`, want: NewSpanContext(SpanContextConfig{ TraceID: TraceID{0x01}, SpanID: SpanID{0x2a}, @@ -772,7 +772,7 @@ func TestSpanContextUnmarshalJSON(t *testing.T) { }, { name: "valid partial SpanContext", - input: `{"TraceID":"01000000000000000000000000000000","SpanID":"2a00000000000000"}`, + input: `{"TraceId":"01000000000000000000000000000000","SpanId":"2a00000000000000"}`, want: NewSpanContext(SpanContextConfig{ TraceID: TraceID{0x01}, SpanID: SpanID{0x2a}, @@ -780,12 +780,12 @@ func TestSpanContextUnmarshalJSON(t *testing.T) { }, { name: "invalid TraceID", - input: `{"TraceID":"00000000000000000000000000000000","SpanID":"2a00000000000000"}`, + input: `{"TraceId":"00000000000000000000000000000000","SpanId":"2a00000000000000"}`, wantErr: true, }, { name: "invalid JSON", - input: `{"TraceID:123}`, + input: `{"TraceId:123}`, wantErr: true, }, } From d0de36104c2aacdca7b4be35afaa2a3f27752b5c Mon Sep 17 00:00:00 2001 From: jackgopack4 Date: Tue, 6 May 2025 10:50:56 -0400 Subject: [PATCH 9/9] add traceflags error const [OTEL-2540] --- trace/trace.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/trace/trace.go b/trace/trace.go index 8e09fe88758..3a6b6dc0403 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -21,6 +21,8 @@ const ( errInvalidSpanIDLength errorConst = "hex encoded span-id must have length equals to 16" errNilSpanID errorConst = "span-id can't be all zero" + + errInvalidTraceFlagsLength errorConst = "trace flags must only be 1 byte" ) type errorConst string @@ -222,7 +224,7 @@ func (tf *TraceFlags) UnmarshalJSON(data []byte) error { return err } if len(decoded) != 1 { - return errInvalidTraceIDLength + return errInvalidTraceFlagsLength } *tf = TraceFlags(decoded[0]) return nil