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

Add prometheus scrape annotations to agent #28

Closed
objectiser opened this issue Sep 13, 2018 · 5 comments · Fixed by #684
Closed

Add prometheus scrape annotations to agent #28

objectiser opened this issue Sep 13, 2018 · 5 comments · Fixed by #684
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed

Comments

@objectiser
Copy link
Contributor

Following on from #27, we should add the prometheus scrape/port annotations to the agent.

Issue with the agent as sidecar is to ensure we don't overwrite similar annotations provided by the service.

@annanay25
Copy link
Member

What would be the solution when the service and the agent would like to expose different ports for Prometheus scraping? @objectiser

@objectiser
Copy link
Contributor Author

Not sure - I created the issue as a reminder to check, but unfortunately haven't yet had time to experiment. There is a prometheus issue discussing the topic with one suggested workaround that requires changing the prometheus scrape config which isn't ideal.

@annanay25
Copy link
Member

I agree @objectiser

Can we change the agent config to publish to the same port as the service?

@objectiser
Copy link
Contributor Author

Don't think we can have two processes using the same port within the same pod.

@jpkrohling jpkrohling added good first issue Good for newcomers enhancement New feature or request help wanted Extra attention is needed and removed good first issue Good for newcomers labels Mar 21, 2019
@jpkrohling
Copy link
Contributor

This should be a good first issue, I think. All work would probably be located in pkg/inject/sidecar.go, most likely inside the decorate() function. In case the deployment already has Prometheus annotations, the new logic would just be skipped. Otherwise, new annotations would be added:

		Annotations: map[string]string{
			"prometheus.io/scrape":    "true",
			"prometheus.io/port":      "5778",
		},

The tests for this should be easy to write as well: a positive case, asserting that a deployment without annotations will get the annotations once the sidecar is injected, and a negative case, where deployments with the annotations won't be touched at all (scrape=false, port=1212, for example)

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 hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants