You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'm seeing high cardinality on the http.server.request.duration metric's server.address attribute on servers exposed to the public internet. (this includes IP addresses, attempted CSRF attacks, attempted XSS reflection attacks, and other strange values) The HTTP semantic conventions say this attribute, among others, has a "requirement level" of Opt-In, meaning that it should only be populated when enabled by user configuration, and instrumentation libraries that don't support configuration should never populate it. (this was changed in open-telemetry/semantic-conventions#109)
Removing this attribute would be sufficient for my purposes, but adding configuration via a builder for trillium_opentelemetry::Metrics should be straightforward enough. I'd be happy to contribute a PR for this.
The text was updated successfully, but these errors were encountered:
It makes sense that unverified client data that could be spurious shouldn't be used for metrics. I'd be open to any of the following, in order of decreasing preference, either written by you if you're keen to do so, or else by me:
Metrics::with_server_address_and_port(self, Fn(&Conn) -> Option<(String, u16)>) that allows users to extract these from an alternate source than Host, with the default being None. The downside of this is that it makes the current behavior harder to opt into, but the upside is it offers options like using the bound port and ip?
Metrics::with_server_address(self, Fn(&Conn) -> Option<String>) and Metrics::with_server_port(self, Fn(&Conn) -> Option<u16>) if maybe people would have an alternate source for one but not the other
Either of the above but without the Options, with the assumption that if a user has access to this information they always have access to it.
Metrics::with_server_address_and_port(self) that opts into the current behavior, with the default being that it is not included
Removing it entirely, possibly to be re-added at some later point
Let me know if you intend to work on this, and I'll PR a fix if not. Either way we should be able to get a release out in short order
Sure, I can send a PR. I think that using the locally bound IP and port would be against the spirit of things; the footnotes link to this section which says the attributes should be set based on request headers. I could see a closure also being useful for filtering to only known-good hosts.
I'm seeing high cardinality on the
http.server.request.duration
metric'sserver.address
attribute on servers exposed to the public internet. (this includes IP addresses, attempted CSRF attacks, attempted XSS reflection attacks, and other strange values) The HTTP semantic conventions say this attribute, among others, has a "requirement level" of Opt-In, meaning that it should only be populated when enabled by user configuration, and instrumentation libraries that don't support configuration should never populate it. (this was changed in open-telemetry/semantic-conventions#109)Removing this attribute would be sufficient for my purposes, but adding configuration via a builder for
trillium_opentelemetry::Metrics
should be straightforward enough. I'd be happy to contribute a PR for this.The text was updated successfully, but these errors were encountered: