Skip to content
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

Have reasonable defaults for the geoipprocessor's context and attribute parameters #37070

Open
bencehornak opened this issue Jan 7, 2025 · 2 comments
Labels
enhancement New feature or request processor/geoip

Comments

@bencehornak
Copy link

Component(s)

processor/geoip

Is your feature request related to a problem? Please describe.

As it came out from this discussion the current defaults for the attributes parameter of the geoipprocessor are not meaningful, because context is set to resource and attributes is set to [source.address, client.address], although these attributes are used to describe spans (i.e. records) and never resources.

Describe the solution you'd like

I would change the defaults the following way:

  • context would default to record, because to me it feels like the primary use case (I think it is seldom that somebody has so many resources that they want to geo code their own IPs, instead of their clients' IPs)
  • and/or attributes would have a default value based on the context:
    • if context is record, then [source.address, client.address]
    • else if context is resource, then [host.ip]

Note: these are breaking changes for users, who are relying on default values of these two fields (however, since the default values are not reasonable as discussed above, I don't think that anybody has ever relied on them).

Describe alternatives you've considered

No response

Additional context

The PR #37008 has implemented the attributes property in a way that its default value is not dependent on the context property.

@bencehornak bencehornak added enhancement New feature or request needs triage New item requiring triage labels Jan 7, 2025
@bencehornak bencehornak changed the title Have reasonable defaults for the geoipprocessor's attribute parameter Have reasonable defaults for the geoipprocessor's context and attribute parameters Jan 7, 2025
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@crobert-1
Copy link
Member

Removing needs triage as this issue was requested to be opened by a code owner. There will still need to be discussion to decide correct default values.

@crobert-1 crobert-1 removed the needs triage New item requiring triage label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request processor/geoip
Projects
None yet
Development

No branches or pull requests

2 participants