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

TargetAllocator for daemonset #1828

Closed
tcolgate opened this issue Jun 14, 2023 · 10 comments · Fixed by #2430
Closed

TargetAllocator for daemonset #1828

tcolgate opened this issue Jun 14, 2023 · 10 comments · Fixed by #2430
Labels
area:collector Issues for deploying collector area:target-allocator Issues for target-allocator

Comments

@tcolgate
Copy link
Contributor

Ideally, when deployed as a daemonset, collectors would scrape the pods running on the node
local to the daemonset instance. It's not obvious from the docs or configs as to if that is achievable
with the current targetAllocator strategies.

Am I missing something? Would you accept a PR that implements this?

@TylerHelmuth TylerHelmuth added area:collector Issues for deploying collector area:target-allocator Issues for target-allocator labels Jun 14, 2023
@m1rp
Copy link

m1rp commented Jun 16, 2023

We're also interested in this.
we've been reading through the docs and tried a few things but couldn't get this to work.

@swiatekm
Copy link
Contributor

I'm curious if your use cases includes Prometheus Custom Resources as well. Do you want each Otel Pod to scrape Pods on the same Node, as defined by a ServiceMonitor or PodMonitor? Or just all Pods on the Node with the right annotation? I believe you can achieve the latter with plain Prometheus configuration by using selectors for a pod role.

@tcolgate
Copy link
Contributor Author

Yeah, we are using ServiceMonitor as it seems to be fairly common for third party charts. Most things I've seen still end up with the older style scrape labels, so I could maybe just use those. I'll have a look around.

@jaronoff97
Copy link
Contributor

yeah this is a good idea, we would ultimately implement this as a new strategy that can only be used when target allocator's mode is daemonset. We would first implement the strategy and then expose it under the new target allocator CRD as part of our v2 milestone.

@jaronoff97 jaronoff97 added this to the v1alpha2 CRD release milestone Nov 28, 2023
@swiatekm
Copy link
Contributor

@jaronoff97 do we need to tie this to v2? It's not a breaking change and doesn't have any dependencies.

@jaronoff97
Copy link
Contributor

Its something I would love to get done in the TA prior to the v2 so that when we make the target allocator CRD it can start with a mode which right now it doesn't have.

@jaronoff97
Copy link
Contributor

@matej-g is going to take this on! Thank you 🙇

@matej-g
Copy link
Contributor

matej-g commented Dec 8, 2023

One more gap I have identified in #2430 (comment) - if we encounter a target we cannot assign to any node, how should we handle it. As @swiatekm-sumo suggested:

  • Ignore these targets
  • Assign them randomly
  • Assign them according to some deterministic scheme (could be what the consistent hashing strategy uses)

What my current implementation doing and what I was leaning towards was to ignore these targets, as I would expect users to use this strategy only scenario where we can detect nodes for targets (i.e. they use this strategy only in conjunction with Prometheus CRs or Kubernetes SD).

@swiatekm
Copy link
Contributor

swiatekm commented Dec 8, 2023

I think ignoring the targets is fine as long as we log a warning about it. Possible edge cases I can see for it are Kubernetes SD roles which don't actually add the node label, like service. These can be used to monitor control plane components exposed as Services, but as long as we document this limitation and log warnings about it, it should be fine.

@jaronoff97
Copy link
Contributor

I agree with @swiatekm-sumo, ignoring targets without this fine. However, we should probably log per job not per target. Logging per target could get very spammy.

The idea of assigning them by using a fallback strategy is an interesting one, but something we should look in to as a follow up IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:collector Issues for deploying collector area:target-allocator Issues for target-allocator
Projects
None yet
6 participants