Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Optionally pass trace context in http response via Server-Timing. #34

Merged
merged 3 commits into from
Jul 9, 2020

Conversation

johnbley
Copy link
Contributor

@johnbley johnbley commented Jul 8, 2020

If SIGNALFX_SERVER_TIMING_CONTEXT is turned on, pass trace context
in traceparent form over a Server-Timing header.

https://www.w3.org/TR/server-timing/
https://www.w3.org/TR/trace-context/#traceparent-header

If SIGNALFX_SERVER_TIMING_CONTEXT is turned on, pass trace context
in traceparent form over a Server-Timing header.

https://www.w3.org/TR/server-timing/
https://www.w3.org/TR/trace-context/#traceparent-header
@johnbley johnbley requested a review from owais July 8, 2020 18:56
contrib/internal/httputil/trace.go Outdated Show resolved Hide resolved
"github.com/signalfx/signalfx-go-tracing/ddtrace"
)

func FormatAsTraceParent(context ddtrace.SpanContext) (string,bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect to return invalid traceparent strings from this func? If not, may be get rid of the bool return val and just return a single string? Caller can then check for traceParent != "" instead of ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only case it would be "invalid" would be if the spanid/traceid are 0. I thought about using "" as the "not a real traceparent" but thought it wasn't very idiomatic go. I like your cleanup of the caller.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. I think it actually might be idiomatic. I don't know. Generally, a second boolean return value is returned by operations that can't distinguish between a "not found" vs "found but was empty string" situations like a map lookup. Other cases generally tend to use a single return value.

@owais
Copy link
Contributor

owais commented Jul 9, 2020

LGTM. Added a couple of minor suggestions.

@johnbley johnbley merged commit bba6a83 into master Jul 9, 2020
@pellared pellared deleted the server_timing_tracecontext branch March 30, 2021 08:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants