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

feat: move default node collector port to avoid conflicts and add setting in config #1618

Merged
merged 15 commits into from
Oct 25, 2024

Conversation

blumamir
Copy link
Collaborator

Since node collector runs as a DaemonSet with host network (so that otlp ports 4317 and 4318 are accessible from the pods exporters), it also shares the ports namespace with the node.

The ports it binds to might conflict with ports of other tools installed in the cluster that does similar things, and in this case data collection will fail to start with error: "listen tcp 0.0.0.0:8888: bind: address already in use".

The collectors currently expose and scrape their own metrics on port 8888 which is good candidate for collistions.

This PR:

  • makes the default port for own telemetry endpoint for node collector 55682 to have low chance of collision with something that already runs on the node (instead of using 8888 as default).
  • Adds an option to odigos config to set the port, in case one needs it hard-coded to a specific value to manage the public ports on the node).

The port to use is written on the collectors group CRD by the scheduler to abstract away the default and odigos config resolving, and allow the resolved value to be more easer to consume.

For the cluster collector, the collector runs as deployment with k8s service, thus we can safely use 8888 without colliding with anything else

Copy link
Contributor

@RonFed RonFed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrote a few small non blocking comments

api/odigos/v1alpha1/collectorsgroup_types.go Show resolved Hide resolved
autoscaler/controllers/gateway/configmap_test.go Outdated Show resolved Hide resolved
k8sutils/pkg/utils/collectorgroup_util.go Outdated Show resolved Hide resolved
k8sutils/pkg/utils/config_util.go Outdated Show resolved Hide resolved
scheduler/controllers/collectorgroups/datacollection.go Outdated Show resolved Hide resolved
k8sutils/pkg/utils/collectorgroup_util.go Show resolved Hide resolved
@blumamir blumamir merged commit 34ca2c3 into odigos-io:main Oct 25, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants