[processor/elasticapm] Add host.ip enrichment using the client info address
#924
Conversation
isaacaflores2
changed the title
host.ip enrichment using the client info address
isaacaflores2
force-pushed
the
processor-host-ip
branch
from
1c6f480 to
54bd1ef
Compare
vigneshshanmugam
left a comment
vigneshshanmugam
left a comment
There was a problem hiding this comment.
Have we thought about how this enrichment would work when using this processor in the EDOT gateway?
| // HostIPEnabled controls whether the `host.ip` resource attribute should be set using client info address. | ||
| // When true, the processor will set the `host.ip` attribute from the client address when | ||
| // the mapping mode is "ecs". Defaults to true. | ||
| HostIPEnabled bool `mapstructure:"host_ip_enabled"` |
There was a problem hiding this comment.
Whats the reasoning behind doing this via a flag vs adding it by default if the Client address exists?
There was a problem hiding this comment.
I added the config since I thought users may not/need this new field and to align with the existing approach in the opentelemtry-lib config where each field enrichment is behind a config.
@vigneshshanmugam good point 👍🏼 I guess this is the reason for which Isaac decided to introduce a flag to add the filed. |
Yeah was thinking the same. Got it 👍🏽
Best we could do here would be to align with APM server as EDOT gateway would just act at its replacement. |
Good point @vigneshshanmugam I did not consider the use of this component running in the elastic agent in either agent or gateway mode. Is the concern that the client address may not be correct in a certain setup? I am working on open-telemetry/opentelemetry-collector#14186 which will provide more flexibility on how the client address is determined. @gregkalapos please review and let us know if you have any concerns with the |
# Conflicts: # processor/elasticapmprocessor/config.go # processor/elasticapmprocessor/processor.go
Apologies I did not see your comment earlier. APM-Server relies on extracting the host.ip from headers and the peer address. So the work in the upstream here will provide the flexibility we need |
Sounds good, thanks for the reference. |
# Conflicts: # processor/elasticapmprocessor/processor.go # processor/elasticapmprocessor/processor_test.go
…rocessorECS`. Removed `elastic_apm`test cases since the data_stream.dataset is asserting in the `ecs` test data
|
There were a few merge conflicts with
|
| Metadata: client.NewMetadata(map[string][]string{"x-elastic-mapping-mode": {"ecs"}}), | ||
| }) | ||
| cfg := createDefaultConfig().(*Config) | ||
| cfg.ServiceNameInDataStreamDataset = true |
There was a problem hiding this comment.
[for reviewer] note: this is the config that was being used in the tests, note the cfg define per test case above. So all outputs included the service specific data_stream.dataset. This is why the service specific name is removed in this output file
| stringValue: exec-456 | ||
| - key: data_stream.dataset | ||
| value: | ||
| stringValue: apm.app.test_service |
There was a problem hiding this comment.
[for reviewer] note: the service specific data_stream.dataset. This is expected and consolidates the test cases under the delete elastic_apm
| stringValue: docker | ||
| - key: data_stream.dataset | ||
| value: | ||
| stringValue: apm.app.test_service |
There was a problem hiding this comment.
[for reviewer] note: the service specific data_stream.dataset. This is expected and consolidates the test cases under the delete elastic_apm
3 participants
Overview
host.ipresource attribute value using the client address fromctx.Client.Info.Addrhost_ip_enabledconfigEdit: I forgot to add that I am working on an upstream issue open-telemetry/opentelemetry-collector#14186 that will provide more flexibility on how the ctx.Client.Info.Addr` is set.Edit: The
ctx.Client.Info.Addrmanipulation will now be implemented as a middleware extension: #930 #936Misc updates
TestProcessorECS