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

Handle OpenTracing key-value logging, finished work in PR #25 #26

Merged
merged 2 commits into from
Sep 26, 2016

Conversation

basvanbeek
Copy link
Member

This PR finishes the work started in PR #25 and handles OpenTracing key-value logging in the same way as Jaeger. If only a single key-value log Field exists with key "event", the payload of the log Field will be used instead of a JSON object with log Fields.

Copy link
Contributor

@bhs bhs left a comment

Choose a reason for hiding this comment

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

(thanks!)

span.SetTag("tag", "value")
span.Finish()
spans := recorder.GetSpans()
assert.Equal(t, 1, len(spans))
assert.Equal(t, 1, len(spans[0].Logs))
assert.Equal(t, "event", spans[0].Logs[0].Event)
assert.Equal(t, "payload", spans[0].Logs[0].Payload)
// XXX: broken tests
Copy link
Contributor

Choose a reason for hiding this comment

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

kill these lines, yes?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes... thanks for catching... updating...

@basvanbeek basvanbeek merged commit 4cc04a1 into openzipkin-contrib:master Sep 26, 2016
@codefromthecrypt
Copy link

we should probably add to the README a big caution on encoding json, as it will screw up other tooling, such as datastore-layer queries which are used in the zipkin ecosystem. Here's a note about this openzipkin/openzipkin.github.io#52 (comment)

Another option is to open a hook for a formatter, similar to most logging libraries, such that people can avoid this problem.

richardmarshall pushed a commit to richardmarshall/zipkin-go-opentracing that referenced this pull request Dec 13, 2016
…_tracing

-totalRequests flag and using net/http/httptrace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants