Enhance resourceprocessor #3
Conversation
| ecsConfig := ecs.CreateDefaultConfig() | ||
| return ecsConfig |
There was a problem hiding this comment.
Just for brevity :)
| ecsConfig := ecs.CreateDefaultConfig() | |
| return ecsConfig | |
| return ecs.CreateDefaultConfig() |
| @@ -97,7 +97,6 @@ func createDefaultConfig() component.Config { | |||
| Detectors: []string{env.TypeStr}, | |||
| HTTPClientSettings: defaultHTTPClientSettings(), | |||
| Override: true, | |||
| Attributes: nil, | |||
There was a problem hiding this comment.
We should not drop this config right away. It should be supported for some time as a deprecated option. See https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/splunkhecexporter/config.go#LL88C2-L88C16 as an example
| "net/http" | ||
| "reflect" | ||
| "regexp" |
There was a problem hiding this comment.
These must be in the first group. Linter will complain :)
| if _, err = d.metadataProvider.InstanceID(ctx); err != nil { | ||
| d.logger.Debug("EC2 metadata unavailable", zap.Error(err)) | ||
| return res, "", nil | ||
| } |
There was a problem hiding this comment.
By mistake I think, brought it back
| attr.PutStr(conventions.AttributeHostType, meta.InstanceType) | ||
| attr.PutStr(conventions.AttributeHostName, hostname) | ||
|
|
||
| setResourceAttributes(dictionary, resourceAttributes, attr) |
There was a problem hiding this comment.
I assume you added this function for easier future maintenance, but setResourceAttributes is hard to read, and we might generate helpers wrapping the if enabled check in the future as it's done for metrics scraping receivers. So I'd suggest keeping it simple for now and writing a bit of boilerplate instead:
if resourceAttributes.CloudAccountID.Enabled {
attr.PutStr(conventions.AttributeCloudAccountID, meta.AccountID)
}
There was a problem hiding this comment.
Thanks! I put this method everywhere, it is really much simpler, as sometimes setting these attributes required some further checks and everything was getting so complicated...
omrozowicz-splunk
force-pushed
the
enhance-resourceprocessor
branch
2 times, most recently
from
802d65f to
1a149b3
Compare
omrozowicz-splunk
force-pushed
the
enhance-resourceprocessor
branch
from
ca806a2 to
3c9f454
Compare
…nfig structure, add and update unit tests, update CHANGELOG, fix copy-paste issues, add DEPRECATION note to attributes option
omrozowicz-splunk
force-pushed
the
enhance-resourceprocessor
branch
from
8f7453c to
655ac75
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
N/A