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/statsd] add attribute for source IP #15290

Closed
seankhliao opened this issue Oct 19, 2022 · 13 comments
Closed

[receiver/statsd] add attribute for source IP #15290

seankhliao opened this issue Oct 19, 2022 · 13 comments
Labels
enhancement New feature or request good first issue Good for newcomers priority:p2 Medium receiver/statsd statsd related issues Stale

Comments

@seankhliao
Copy link
Contributor

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

We want to enrich the metrics data we receive over the statsd protocol via the k8sattributes processor.
This requires having something to identify the source of metrics, either IP address, name, or uid.

Using the source IP address seems ideal as it doesn't require cooperation from the process producing the metrics.

Describe the solution you'd like

An attribute containing the source IP of the packet each metric came from.

Describe alternatives you've considered

Getting producers to include self identifying data: not always easy/possible
Run collector as sidecar: resource expensive and hard to coordinate well (eg k8s Jobs)

Additional context

No response

@seankhliao seankhliao added enhancement New feature or request needs triage New item requiring triage labels Oct 19, 2022
@evan-bradley evan-bradley added priority:p2 Medium receiver/statsd statsd related issues and removed needs triage New item requiring triage labels Oct 19, 2022
@github-actions
Copy link
Contributor

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

@jmacd jmacd added the good first issue Good for newcomers label Oct 24, 2022
@seankhliao
Copy link
Contributor Author

Thinking about doing this, do I need to make the attribute the peer address configurable?
net.sock.peer.addr seems to be the semconv name for the address, but there's the risk of overwriting an existing attribute.

@dmitryax
Copy link
Member

dmitryax commented Nov 4, 2022

@seankhliao Maybe instead of introducing another attribute, we can follow the same approach as done for tracing where the source ip is passed in a client request context. Please take a look into that

@seankhliao
Copy link
Contributor Author

Once parsed, the statsd receiver mixes everything into a few maps for aggregation. The metrics are only passed on the next consumer on flush intervals disassociated with the incoming requests.
I don't really see a place to store the connection context, or a way to group the metrics by context before passing to the next consumer.

@dmitryax
Copy link
Member

dmitryax commented Nov 8, 2022

Ok makes sense, it probably should be set as a resource attribute in that case, maybe configured in settings. Not sure about using net.sock.peer.addr since it's a span attribute. Should it be a resource attribute? Does statsd metrics reporter send metrics about itself or not necessary?

@seankhliao
Copy link
Contributor Author

The statsd reporters I've seen are usually unaware of their own identity.
Resource attribute would make sense except that I don't think there's a place to store resource attributes? unless we add a new member to https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/statsdreceiver/protocol/statsd_parser.go#L119
Per metric attribute will probably work if combined with the groupbyattribute processor.
Didn't notice net.sock.peer.addr is only for spans, a configurable key sounds like the best option then.

@jsirianni
Copy link
Member

I would love to have this. Right now I am trying to combine statsd with Kubernetes Attributes Processor, which can use a pod ip address to lookup additional metadata, like pod name and such. This would be useful when using statsd in a kubernetes environment.

@jsirianni
Copy link
Member

Additionally, it would be nice if the resource key was configurable. the K8s attributes processor can look for k8s.pod.ip, which would mean renaming net.sock.peer.addr (or whatever ends up being implemented. Not a big deal.

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@seankhliao
Copy link
Contributor Author

I had a go at implementing it with a configurable key in #18276

Though it uses an entire address (ip:port for UDP), so users might need to run another processor to split out a plain IP if downstream (eg k8sattributesprocessor) needs that.

@jmacd
Copy link
Contributor

jmacd commented Mar 13, 2023

#18276 (comment)

codeboten pushed a commit that referenced this issue Mar 15, 2023
A new config key source_address_key, if set, will be used to record the address of the sender of incoming metrics.

Link to tracking Issue: #15290
@github-actions github-actions bot removed the Stale label May 27, 2023
@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • issue: Github issue template generation code needs this to generate the corresponding labels.
  • receiver/statsd: @jmacd @dmitryax

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

@github-actions github-actions bot added the Stale label Jul 26, 2023
@codeboten
Copy link
Contributor

This was addressed by #18276

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers priority:p2 Medium receiver/statsd statsd related issues Stale
Projects
None yet
Development

No branches or pull requests

6 participants