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

RFC: add SpanContext.LocalRootSpanID #1029

Merged
merged 3 commits into from
Feb 14, 2019

Conversation

axw
Copy link
Contributor

@axw axw commented Feb 12, 2019

This is a companion to census-instrumentation/opencensus-specs#229. These are the changes that would be necessary in order for Elastic APM to implement an exporter, as in elastic/apm-agent-go@master...axw:opencensus.

I'd appreciate feedback either here or on the spec issue, indicating whether this might be acceptable.

axw added 2 commits February 12, 2019 17:02
SpanContext should only hold things that are
propagated between processes, which is not the
case for this field.
trace/trace_test.go Outdated Show resolved Hide resolved
trace/trace_test.go Outdated Show resolved Hide resolved
@rghetia rghetia merged commit 57c0993 into census-instrumentation:master Feb 14, 2019
@axw
Copy link
Contributor Author

axw commented Feb 15, 2019

Thanks @rghetia.

I hadn't intended for this to be merged before census-instrumentation/opencensus-specs#229 was resolved, but if you're happy for this to go in then I'm happy.

@rghetia
Copy link
Contributor

rghetia commented Feb 15, 2019

Thanks @rghetia.

I hadn't intended for this to be merged before census-instrumentation/opencensus-specs#229 was resolved, but if you're happy for this to go in then I'm happy.

It looks fine to me however I should not have merged until the spec is modified first. I'll roll it back unless we decide to update the spec based on this PR.

@tsloughter
Copy link
Member

Is this being added to the actual proto/spec? Right now it means the opencensus go deviates from the other implementations and they won't be able to work elastic.

rghetia added a commit to rghetia/opencensus-go that referenced this pull request Feb 20, 2019
rghetia added a commit that referenced this pull request Feb 20, 2019
This reverts commit 57c0993.

It was merged without updating the spec and resolving
census-instrumentation/opencensus-specs#229
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.

3 participants