Add SAPM AccessTokenPassthrough #349
Conversation
Codecov Report
@@ Coverage Diff @@
## master #349 +/- ##
==========================================
+ Coverage 82.78% 83.15% +0.36%
==========================================
Files 170 170
Lines 9070 9116 +46
==========================================
+ Hits 7509 7580 +71
+ Misses 1236 1217 -19
+ Partials 325 319 -6
Continue to review full report at Codecov.
|
| for i := 0; i < resourceSpans.Len(); i++ { | ||
| resourceSpan := resourceSpans.At(i) | ||
| if resourceSpan.IsNil() { | ||
| // Invalid trace so nothing to export |
There was a problem hiding this comment.
Not sure about hits. @pjanotti do we need to export empty Span slice to make sure our metrics are correct?
There was a problem hiding this comment.
This should be fine: we want the exported metrics to reflect the actual number of exported spans and since a nil span is not exported that is correct.
@rmfitzpatrick just double checking: this is just defensive programming, right? or did you encounter it in practice?
There was a problem hiding this comment.
Correct, I've not run into this being the case but wanted to ensure I wouldn't cause panics: https://github.com/open-telemetry/opentelemetry-collector/blob/a8db6271f90ad6877e77967cfaf288cda6e810bd/consumer/pdata/generated_trace.go#L172
exporter/sapmexporter/exporter.go
Outdated
| // tracesByAccessToken takes a pdata.Traces struct and will iterate through its ResourceSpans' attributes, | ||
| // regrouping by any SFx access token label value if Config.AccessTokenPassthrough is enabled. It will drop any | ||
| // set token label in any case to prevent serialization. | ||
| // It returns a map of newly constructed pdata.Traces keyed by access token, defaulting to empty string. |
exporter/sapmexporter/exporter.go
Outdated
| if err != nil { | ||
| return td.SpanCount(), consumererror.Permanent(err) | ||
| // tracesByAccessToken takes a pdata.Traces struct and will iterate through its ResourceSpans' attributes, | ||
| // regrouping by any SFx access token label value if Config.AccessTokenPassthrough is enabled. It will drop any |
There was a problem hiding this comment.
| // regrouping by any SFx access token label value if Config.AccessTokenPassthrough is enabled. It will drop any | |
| // regrouping by any SFx access token label value if Config.AccessTokenPassthrough is enabled. It will delete any |
| continue | ||
| } | ||
|
|
||
| exportErr := se.client.ExportWithAccessToken(ctx, batches, accessToken) |
There was a problem hiding this comment.
If accessToken is "" is the default access token used?
There was a problem hiding this comment.
Right, this occurs in the updated client's worker: https://github.com/signalfx/sapm-proto/blob/6b28b8974508e838a308eb9fbf1e624b84c67132/client/worker.go#L116
|
@rmfitzpatrick please resolve the conflicts. |
Done |
Adding the `access_token_passthrough` config option and functionality to SAPM receiver and exporter similar to that of the SignalFx metric receiver and exporter from f361e4d. These changes add a `com.splunk.signalfx.access_token` attribute with a value taken from `X-Sf-Token` headers, if provided, that will be used for subsequent trace submissions in the SAPM exporter by its client. **Testing:** Added and modified unit tests and e2e run configuration. **Documentation:** Updated relevant readmes.
* Bump Jaeger version, refactor jaegerreceiver and healthcheck Signed-off-by: Annanay <[email protected]> * Run gofmt Signed-off-by: Annanay <[email protected]> * Fix goimports error Signed-off-by: Annanay <[email protected]> * Fix healthcheck tests, refactor code Signed-off-by: Annanay <[email protected]> * Fix healthcheck Signed-off-by: Annanay <[email protected]> * Address review comments Signed-off-by: Annanay <[email protected]>
* Remove AddLink & Link from Span Interface I have remove AddLink and Link from the interface and all it refereneces and replaced AddLink with addlink, Also Removed respective unit tests Signed-off-by: vineeth <[email protected]> * removing the unused code from unit tests Signed-off-by: VineethReddy02 <[email protected]>
Co-authored-by: alrex <[email protected]>
Description: Adding the
access_token_passthroughconfig option and functionality to SAPM receiver and exporter similar to that of the SignalFx metric receiver and exporter from f361e4d. These changes add acom.splunk.signalfx.access_tokenattribute with a value taken fromX-Sf-Tokenheaders, if provided, that will be used for subsequent trace submissions in the SAPM exporter by its client.Testing: Added and modified unit tests and e2e run configuration.
Documentation: Updated relevant readmes.