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

implemented JSON distributed trace converter #335

Merged
merged 4 commits into from
Jul 1, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions v3/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ module github.com/newrelic/go-agent/v3
go 1.7

require (
github.com/golang/protobuf v1.3.3
google.golang.org/grpc v1.27.0
github.com/golang/protobuf v1.4.3
google.golang.org/grpc v1.39.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're requiring new versions of these libraries, will this affect our customers in any way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can move the version back. That was automatically bumped by my go runtime, not because I purposefully upgraded it.

)
42 changes: 42 additions & 0 deletions v3/newrelic/trace_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"net/http"
"reflect"
"strings"
"testing"

Expand Down Expand Up @@ -46,6 +47,47 @@ type TraceContextTestCase struct {
} `json:"intrinsics"`
}

func TestJsonDTHeaders(t *testing.T) {
type testcase struct {
in string
out http.Header
err bool
}

for i, test := range []testcase{
{"", http.Header{}, false},
{"{}", http.Header{}, false},
{" invalid ", http.Header{}, true},
{`"foo"`, http.Header{}, true},
{`{"foo": "bar"}`, map[string][]string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this map[string][]string rather than just another http.Header? I've changed them back to http.Header and all seems to work fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http.Header is more correct to use, even though they're (currently) equivalent.

"Foo": {"bar"},
}, false},
{`{
"foo": "bar",
"baz": "quux",
"multiple": [
"alpha",
"beta",
"gamma"
]
}`, map[string][]string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here with http.Header vs. map[string][]string

"Foo": {"bar"},
"Baz": {"quux"},
"Multiple": {"alpha", "beta", "gamma"},
}, false},
} {
h, err := DistributedTraceHeadersFromJson(test.in)

if err != nil {
if !test.err {
t.Errorf("case %d: %v: error expected but not generated", i, test.in)
}
} else if !reflect.DeepEqual(test.out, h) {
t.Errorf("case %d, %v -> %v but expected %v", i, test.in, h, test.out)
}
}
}

func TestCrossAgentW3CTraceContext(t *testing.T) {
var tcs []TraceContextTestCase

Expand Down
72 changes: 72 additions & 0 deletions v3/newrelic/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package newrelic

import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -269,6 +271,76 @@ func (txn *Transaction) AcceptDistributedTraceHeaders(t TransportType, hdrs http
txn.thread.logAPIError(txn.thread.AcceptDistributedTraceHeaders(t, hdrs), "accept trace payload", nil)
}

//
// AcceptDistributedTraceHeadersFromJson() works just like AcceptDistributedTraceHeaders(), except
// that it takes the header data as a JSON string à la DistributedTraceHeadersFromJson(). Additionally
// (unlike AcceptDistributedTraceHeaders()) it returns an error if it was unable to successfully
// convert the JSON string to http headers. There is no guarantee that the header data found in JSON
// is correct beyond conforming to the expected types and syntax.
//
func (txn *Transaction) AcceptDistributedTraceHeadersFromJson(t TransportType, jsondata string) error {
hdrs, err := DistributedTraceHeadersFromJson(jsondata)
if err != nil {
return err
}
txn.AcceptDistributedTraceHeaders(t, hdrs)
return nil
}

//
// DistributedTraceHeadersFromJson() takes a set of distributed trace headers as a JSON-encoded string
// and emits a http.Header value suitable for passing on to the
// txn.AcceptDistributedTraceHeaders() function.
//
// For example, given the input string
// `{"traceparent": "frob", "tracestate": "blorfl", "newrelic": "xyzzy"}`
// This will emit an http.Header value with headers "traceparent", "tracestate", and "newrelic".
//
// This is a convenience function provided for cases where you receive the trace header data
// already as a JSON string and want to avoid manually converting that to an http.Header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we want to add that this is likely when working with trace headers with different language agents? Maybe that would be too wordy. I just want to make sure somebody like Jo Ann would see this and use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea. I don't mind leaning to more verbosity when it comes to documentation. You never know when that extra sentence or paragraph was just what someone was looking for.

//
// The JSON string must be a single object whose values may be strings or arrays of strings.
// These are translated directly to http headers with singleton or multiple values.
//
func DistributedTraceHeadersFromJson(jsondata string) (hdrs http.Header, err error) {
var raw interface{}
hdrs = http.Header{}
if jsondata == "" {
RichVanderwal marked this conversation as resolved.
Show resolved Hide resolved
return
}
err = json.Unmarshal([]byte(jsondata), &raw)
if err != nil {
return
}

switch d := raw.(type) {
case map[string]interface{}:
for k, v := range d {
switch hval := v.(type) {
case string:
hdrs.Set(k, hval)
case []interface{}:
for _, subval := range hval {
switch sval := subval.(type) {
case string:
hdrs.Add(k, sval)
default:
err = fmt.Errorf("JSON object must have only strings or arrays of strings.")
return
}
}
default:
err = fmt.Errorf("JSON object must have only strings or arrays of strings.")
return
}
}
default:
err = fmt.Errorf("JSON string must be a single object.")
return
}
return
}

// Application returns the Application which started the transaction.
func (txn *Transaction) Application() *Application {
if nil == txn {
Expand Down