Add common signalfx host id extraction #1100
Conversation
| @@ -416,6 +416,7 @@ func float64ToDimValue(f float64) string { | |||
| // resource attributes, including a cloud host id (AWSUniqueId, gcp_id, etc.) | |||
| // if it can be constructed from the provided metadata. | |||
| func appendResourceAttributesToDimensions(dims []*sfxpb.Dimension, resourceAttr map[string]string) []*sfxpb.Dimension { | |||
| // TODO: Replace with internal/common/splunk/hostid.go once signalfxexporter is converted to pdata. | |||
There was a problem hiding this comment.
Are you going to still remove the resource attributes used to derive the host id like the code below in this function does? If so, it seems like there will need to be something more that the hostid module does to facilitate that instead of having to know the exact fields here.
There was a problem hiding this comment.
Nope, sapm won't remove them since for traces we want all those tags. I think signalfxexporter will probably need to maintain its own list of labels/dimensions to filter out. It doesn't need to be per-provider type like it is today, it can just delete all of them (provider, account id, region, zone, etc. regardless of provider type).
This adds a shared way to extract AWSUniqueId and gcp_id dimensions. It will be used by both sapmexporter and signalfxexporter. signalfxexporter cannot currently use the method because it's not on pdata and it seemed silly to unconvert metrics data back to pdata. I've added a note to signalfxexporter to use this common implementation once it switches to pdata.
Codecov Report
@@ Coverage Diff @@
## master #1100 +/- ##
==========================================
+ Coverage 89.01% 89.04% +0.02%
==========================================
Files 260 261 +1
Lines 12413 12438 +25
==========================================
+ Hits 11050 11075 +25
Misses 1011 1011
Partials 352 352
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| ProviderAWS = "aws" | ||
| // ProviderGCP is used in cloud.provider label for GCP. | ||
| ProviderGCP = "gcp" |
There was a problem hiding this comment.
Should this be in core semantic conventions or this is Splunk convention only?
There was a problem hiding this comment.
aws and gcp are called out as examples in the conventions (https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/resource/semantic_conventions/cloud.md). Should I create a PR to amend semantic conventions to make it explicit that these values should be used for the platforms? If so I think then it'd make sense to put this in translator/conventions/opentelemetry.go.
There was a problem hiding this comment.
Yes, best to add to core conventions.
|
Ok let's do that, will open PR to spec to add to conventions and update here once it lands. |
|
|
@tigrannajaryan @bogdandrutu can we merge this? I have a PR which depends on it |
…#1100) Since OTLP span kinds has 1-1 mapping with OpenTracing specification we don't need to keep additional "span.kind" value as was done in OC internal representation. The commit fixes the issue: open-telemetry/opentelemetry-collector#878
Add check for github-actions. Add missing examples and SDK go.mod Remove redundant comments. Change check to be weekly on Sunday to reduce load and churn. Sort alphanumerically. Add check to Makefile to ensure if there is a `go.mod` file there is a dependabot entry for that directory.
The failing test was introduced I believe by a change in behaviour noted in this issue: getmoto/moto#3292. The solution is to set a region in the create_bucket call. Fixes #1088
The failing test was introduced I believe by a change in behaviour noted in this issue: getmoto/moto#3292. The solution is to set a region in the create_bucket call. Fixes #1088
This adds a shared way to extract AWSUniqueId and gcp_id dimensions. It will be
used by both sapmexporter and signalfxexporter. signalfxexporter cannot
currently use the method because it's not on pdata and it seemed silly to
unconvert metrics data back to pdata. I've added a note to signalfxexporter to
use this common implementation once it switches to pdata.