-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[processor/geoipprocessor] Add attributes
parameter and consider both source.address
and client.address
by default
#37008
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The client.address
attribute closely resembles source.address
, so I support adding it to the default lookup attributes. (It would be great to have a configuration option to allow any arbitrary attribute key).
My assumption is that if both attributes are defined, they will likely have the same value, making the lookup order less critical.
client.address
and the source.address
attributesattributes
parameter and consider both source.address
and client.address
by default
@rogercoll I've implemented a new configuration parameter ( |
I think the I could imagine the Thanks in advance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added some nits, overall LGTM!
For example, the
That's a good point, would you mind creating an issue with these findings? We can definitely consider changing the attributes defaults depending on the signal type. |
Description
Currently, the
geoipprocessor
reads thesource.address
attribute of the processed record, which is used to describe network exchanges/packets only (see the semantic conventions). For HTTP requests there is another attribute, calledclient.address
with the same format (see the semantic conventions).In this PR I am changing the geoipprocessor take both attributes into account by default (if both are defined
client.attributes
wins). Additionally, the attributes are configurable via the newattributes
parameter to allow geo ip lookup based on custom attributes.Testing
I duplicated the
source address located in inner attributes
test case forclient.address
, as they should work the same way.Documentation
Updated the README with the second attribute.