opencensus-go-exporter-stackdriver
opencensus-go-exporter-stackdriver copied to clipboard
update SpanID type in http propagation to adhere to trace API V2 spec
According to https://cloud.google.com/trace/docs/reference/v2/rest/v2/projects.traces/batchWrite
[SPAN_ID] is a unique identifier for a span within a trace; it is a 16-character hexadecimal encoding of an 8-byte array.
Codecov Report
Merging #254 into master will decrease coverage by
0.01%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #254 +/- ##
==========================================
- Coverage 72.02% 72.01% -0.02%
==========================================
Files 17 17
Lines 1691 1690 -1
==========================================
- Hits 1218 1217 -1
Misses 397 397
Partials 76 76
Impacted Files | Coverage Δ | |
---|---|---|
propagation/http.go | 60.00% <100.00%> (-1.30%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update e191b7c...5ff15ed. Read the comment docs.
@rghetia @bogdandrutu could one of you gents please take a look at this when you get a chance?
@rghetia @bogdandrutu could one of you gents please take a look at this when you get a chance?
@anovitskiy thanks for the changes. Changes look good however I have one question. Would there be a case where a user would need to use v1 format?
@rghetia @bogdandrutu could one of you gents please take a look at this when you get a chance?
@anovitskiy thanks for the changes. Changes look good however I have one question. Would there be a case where a user would need to use v1 format?
Thanks, @rghetia Luckily, this is completely backwards compatible since an 8-byte unsigned integer can be encoded into hexadecimal and decoded back to the same value.
@rghetia @bogdandrutu could one of you gents please take a look at this when you get a chance?
@anovitskiy thanks for the changes. Changes look good however I have one question. Would there be a case where a user would need to use v1 format?
Thanks, @rghetia Luckily, this is completely backwards compatible since an 8-byte unsigned integer can be encoded into hexadecimal and decoded back to the same value.
What I meant was that the new format is encoding spanid into a hex string but if on the other side if it is expecting decimal string then it won't work.
@rghetia @bogdandrutu could one of you gents please take a look at this when you get a chance?
@anovitskiy thanks for the changes. Changes look good however I have one question. Would there be a case where a user would need to use v1 format?
Thanks, @rghetia Luckily, this is completely backwards compatible since an 8-byte unsigned integer can be encoded into hexadecimal and decoded back to the same value.
What I meant was that the new format is encoding spanid into a hex string but if on the other side if it is expecting decimal string then it won't work.
Hopefully for most users the calls to SpanContextToRequest() and SpanContextFromRequest() are in the same library but the release version should indicate that this is a potentially breaking change. Open to ideas on how else to version this change.
Hopefully for most users the calls to SpanContextToRequest() and SpanContextFromRequest() are in the same library but the release version should indicate that this is a potentially breaking change. Open to ideas on how else to version this change.
Breaking change is not desirable at this point given that opencensus is replaced by opentelemetry. One option is to
- Add version field in HTTPFormat and a new func to create version 2 object. Use conditional encoding/decoding based on the version.
type HTTPFormat struct{
version int
}
const (
httpFormatVersion2 = 2
)
func NewHTTPFormatV2() HTTPFormat {
return HTTPFormat{
version: httpFormatVersion2,
}
}
the other option is to 'With*' style option to configure
func NewHTTPFormat(o Options...) {
}
Options approach may be too much as I don't expect more options in the future.
A third option would be to preserve the original trace and span IDs as strings instead of decoding and encoding. Was the original design chosen in order to conserve memory?