Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apm: fix span event msgpk deserialization, add test #35033

Merged
merged 3 commits into from
Mar 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions cmd/trace-agent/test/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,25 @@ func (s *Runner) BinDir() string {
//
// Example: r.PostMsgpack("/v0.5/stats", pb.ClientStatsPayload{})
func (s *Runner) PostMsgpack(path string, data msgp.Marshaler) (err error) {
var b []byte
if b, err = data.MarshalMsg(nil); err != nil {
return err
}
return s.PostBinary(path, b)
}

// PostBinary takes messagepack encoded data and posts it to the given path. The agent
// must be started using RunAgent.
//
// Example: r.PostBinary("/v0.5/traces", []byte])
func (s *Runner) PostBinary(path string, data []byte) (err error) {
if s.agent == nil {
return ErrNotStarted
}
if s.agent.PID() == 0 {
return errors.New("post: trace-agent not running")
}
var b []byte
if b, err = data.MarshalMsg(nil); err != nil {
return err
}
buf := bytes.NewBuffer(b)
buf := bytes.NewBuffer(data)
addr := fmt.Sprintf("http://%s%s", s.agent.Addr(), path)
req, err := http.NewRequest("POST", addr, buf)
if err != nil {
Expand Down
Binary file not shown.
35 changes: 34 additions & 1 deletion cmd/trace-agent/test/testsuite/traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package testsuite

import (
_ "embed"
"strings"
"testing"
"time"
Expand All @@ -21,6 +22,9 @@ import (
// create a new config to access default config values
var defaultAgentConfig = config.New()

//go:embed testdata/v04SpanEvents.msgp
var rubySpanEventsPayload []byte

func TestTraces(t *testing.T) {
var r test.Runner
if err := r.Start(); err != nil {
Expand All @@ -41,7 +45,7 @@ func TestTraces(t *testing.T) {
p := testutil.GeneratePayload(10, &testutil.TraceConfig{
MinSpans: 10,
Keep: true,
}, nil)
}, &testutil.SpanConfig{NumSpanEvents: 1})
if err := r.Post(p); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -229,6 +233,26 @@ func TestTraces(t *testing.T) {
payloadsEqual(t, p, v)
})
})

t.Run("ruby-span-events", func(t *testing.T) {
if err := r.RunAgent([]byte("apm_config:\r\n env: my-env")); err != nil {
t.Fatal(err)
}
defer r.KillAgent()

if err := r.PostBinary("/v0.4/traces", rubySpanEventsPayload); err != nil {
t.Fatal(err)
}
waitForTrace(t, &r, func(v *pb.AgentPayload) {
assert.Len(t, v.TracerPayloads[0].Chunks[0].Spans[0].SpanEvents, 1)
spanEvent := v.TracerPayloads[0].Chunks[0].Spans[0].SpanEvents[0]
assert.Equal(t, "event-name", spanEvent.Name)
assert.NotZero(t, spanEvent.TimeUnixNano)
assert.Len(t, spanEvent.Attributes, 1)
assert.Equal(t, pb.AttributeAnyValue_AttributeAnyValueType(0), spanEvent.Attributes["key"].Type)
assert.Equal(t, "value", spanEvent.Attributes["key"].StringValue)
})
})
}

// payloadsEqual validates that the traces in from are the same as the ones in to.
Expand Down Expand Up @@ -304,5 +328,14 @@ func spansEqual(s1, s2 *pb.Span) bool {
return false
}
}
if len(s1.SpanEvents) != len(s2.SpanEvents) {
return false
}
for i, se := range s1.SpanEvents {
if se.Name != s2.SpanEvents[i].Name ||
se.TimeUnixNano != s2.SpanEvents[i].TimeUnixNano {
return false
}
}
return true
}
22 changes: 17 additions & 5 deletions pkg/proto/datadog/trace/span.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ message SpanLink {
}

message SpanEvent {
// time is the number of nanoseconds between the Unix epoch and this event.
fixed64 time_unix_nano = 1;
// name is this event's name.
string name = 2;
// @gotags: json:"time_unix_nano" msg:"time_unix_nano"
fixed64 time_unix_nano = 1; // time is the number of nanoseconds between the Unix epoch and this event.
// @gotags: json:"name" msg:"name"
string name = 2; // name is this event's name.
// attributes is a mapping from attribute key string to any value.
// The order of attributes should be preserved in the key/value map.
// The supported values match the OpenTelemetry attributes specification:
// https://github.com/open-telemetry/opentelemetry-proto/blob/a8f08fc49d60538f97ffabcc7feac92f832976dd/opentelemetry/proto/common/v1/common.proto
// @gotags: json:"attributes" msg:"attributes"
map <string,AttributeAnyValue> attributes = 3;
}

Expand All @@ -39,6 +40,7 @@ message AttributeAnyValue {
// Protobuf `oneof` unions: https://github.com/tinylib/msgp/issues/184
// Despite this, the format represented here is binary compatible with `oneof`, if we choose
// to migrate to that in the future.
// @gotags: json:"type" msg:"type"
AttributeAnyValueType type = 1;

enum AttributeAnyValueType {
Expand All @@ -48,18 +50,23 @@ message AttributeAnyValue {
DOUBLE_VALUE = 3;
ARRAY_VALUE = 4;
}

// @gotags: json:"string_value" msg:"string_value"
string string_value = 2;
// @gotags: json:"bool_value" msg:"bool_value"
bool bool_value = 3;
// @gotags: json:"int_value" msg:"int_value"
int64 int_value = 4;
// @gotags: json:"double_value" msg:"double_value"
double double_value = 5;
// @gotags: json:"array_value" msg:"array_value"
AttributeArray array_value = 6;
}


// AttributeArray is a list of AttributeArrayValue messages. We need this as a message since `oneof` in AttributeAnyValue does not allow repeated fields.
message AttributeArray {
// Array of values. The array may be empty (contain 0 elements).
// @gotags: json:"values" msg:"values"
repeated AttributeArrayValue values = 1;
}

Expand All @@ -70,6 +77,7 @@ message AttributeArrayValue {
// Protobuf `oneof` unions: https://github.com/tinylib/msgp/issues/184
// Despite this, the format represented here is binary compatible with `oneof`, if we choose
// to migrate to that in the future.
// @gotags: json:"type" msg:"type"
AttributeArrayValueType type = 1;

enum AttributeArrayValueType {
Expand All @@ -79,9 +87,13 @@ message AttributeArrayValue {
DOUBLE_VALUE = 3;
}

// @gotags: json:"string_value" msg:"string_value"
string string_value = 2;
// @gotags: json:"bool_value" msg:"bool_value"
bool bool_value = 3;
// @gotags: json:"int_value" msg:"int_value"
int64 int_value = 4;
// @gotags: json:"double_value" msg:"double_value"
double double_value = 5;
}

Expand Down
47 changes: 30 additions & 17 deletions pkg/proto/pbgo/trace/span.pb.go

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

Loading