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

[receiver/apache] send port number as an attribute #14791

Closed
aboguszewski-sumo opened this issue Oct 10, 2022 · 6 comments · Fixed by #17355
Closed

[receiver/apache] send port number as an attribute #14791

aboguszewski-sumo opened this issue Oct 10, 2022 · 6 comments · Fixed by #17355
Labels

Comments

@aboguszewski-sumo
Copy link
Member

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

Currently, the receiver only sends the server's name (which is the hostname) as an attribute. It would be useful to send also the port (for example, Sumo Logic app for Apache uses it).
The important thing to note here is that the port can't be deduced solely from the endpoint - the default port is different when we use http (80) and when we use https (443).

Describe the solution you'd like

An additional attribute should be sent with the metrics.
The solution will be short and easy to implement, here is how Telegraf does it: https://github.com/influxdata/telegraf/blob/master/plugins/inputs/apache/apache.go#L190

Describe alternatives you've considered

No response

Additional context

The hostname is currently being sent as a separate attribute, not a resource attribute - it would be cool to change that, but I'm not sure if it should be done in the same PR. I'm not sure, how to add the port here. Ideally, I think that both attributes should be resource attributes. If we shouldn't change now how the hostname is sent, should the port be sent as a normal attribute (to keep the convention) or as a resource attribute (as it actually should be)?

@aboguszewski-sumo aboguszewski-sumo added enhancement New feature or request needs triage New item requiring triage labels Oct 10, 2022
@evan-bradley evan-bradley added priority:p2 Medium receiver/apache and removed needs triage New item requiring triage labels Oct 10, 2022
@github-actions
Copy link
Contributor

Pinging code owners: @djaglowski. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@djaglowski
Copy link
Member

I agree that both hostname and port should be a resource attributes. The changes should be made in separate PRs, but we should try to get them both into the same release.

@aboguszewski-sumo
Copy link
Member Author

If I'm not mistaken, the next release (0.62) will be this Friday? Since Elasticsearch metrics have more priority for now, I will add these changes for 0.63.

@aboguszewski-sumo
Copy link
Member Author

I've created a commit for port as well, I will rebase it and create a PR as soon as #14926 gets merged.

@aboguszewski-sumo
Copy link
Member Author

The changes have been merged, but they are behind feature gates. I don't know what's the policy here, but I suggest to leave this issue open until the feature gates are gone.

@aboguszewski-sumo
Copy link
Member Author

This will be resolved once #17355 gets merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants