[FEAT] edit w3c tracing context configurations on account and enable tracing into service exports #638
Conversation
…tracing into service exports
There was a problem hiding this comment.
Some changes needed, but also you need to add the allow trace for the stream import (with similar checks that will fail allow-trace on service import). See JWT pr for details: nats-io/jwt#216
cmd/editaccount.go
Outdated
| } else { | ||
| p.claim.Trace = &jwt.MsgTrace{} | ||
| p.claim.Trace.Destination = jwt.Subject(p.traceContextSubject) | ||
| r.AddOK("changed trace context subject to %d", p.claim.Trace.Sampling) |
There was a problem hiding this comment.
You are supposed to show the change of the subject, but you use %d and show the sampling value.
cmd/editaccount.go
Outdated
| } | ||
| // we already validated that the subject is there either existing or new | ||
| p.claim.Trace.Sampling = int(p.traceContextSampling.Int64()) | ||
| r.AddOK("changed trace context sampling to %d", p.claim.Trace.Sampling) |
There was a problem hiding this comment.
You could add to %d%% to show it as a percentage, but it is ok this way too.
| @@ -542,7 +552,10 @@ func (p *AddImportParams) createImport() *jwt.Import { | |||
| if p.service { | |||
| im.Type = jwt.Service | |||
| im.Share = p.share | |||
| } else if p.allowTrace { | |||
| im.AllowTrace = true | |||
There was a problem hiding this comment.
Are you sure it would not be im.AllowTrace = p.allowTrace instead of true?
There was a problem hiding this comment.
yes, the other functions are managing the flags, the import creation happens in createImport() which then sets it to be as it needs to be.
There was a problem hiding this comment.
Oh sorry... I saw the else of the if p.service and I thought you were just processing stream properties here and always setting to true without seeing that the else if p.allowTrace. My bad.
| @@ -542,7 +552,10 @@ func (p *AddImportParams) createImport() *jwt.Import { | |||
| if p.service { | |||
| im.Type = jwt.Service | |||
| im.Share = p.share | |||
| } else if p.allowTrace { | |||
| im.AllowTrace = true | |||
There was a problem hiding this comment.
Oh sorry... I saw the else of the if p.service and I thought you were just processing stream properties here and always setting to true without seeing that the else if p.allowTrace. My bad.
Fix #636