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

config: Validate duplicate job names #3986

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

nabokihms
Copy link
Contributor

This commit fixes the problem when jobs with the same names are compacted to a single target, leading to unexpected behavior: a user will only see the last target with this job name.

It is a breaking change for users who have duplicates in their configs, but for new users, it will be more transparent.

@nabokihms nabokihms requested a review from a team as a code owner October 26, 2023 09:29
@CLAassistant
Copy link

CLAassistant commented Oct 26, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

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

Nice find!

@brancz
Copy link
Member

brancz commented Oct 26, 2023

Can you try running the benchmarks? They seem to fail consistently on CI.

This commit fixes the problem when jobs with the same names are compacted to a single target, leading to an unexpected behavior: a user will only see the last target with this job name.

It is a breaking change for users who have duplicates in their configs, but for new users it will be more transparent.
This test is needed to validate that the case from the doc is working (desired parca + parca-agent installation).
@nabokihms nabokihms force-pushed the validate-duplicate-job-names branch from c731278 to 845c442 Compare October 26, 2023 18:55
@nabokihms
Copy link
Contributor Author

@brancz sorry, it was my mistake. I accidentally made the scrape config required.

@brancz
Copy link
Member

brancz commented Oct 27, 2023

Thank you so much for the contribution!

@manojVivek
Copy link
Contributor

@all-contributors Please add @nabokihms for code.

@allcontributors
Copy link
Contributor

@manojVivek

I've put up a pull request to add @nabokihms! 🎉

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.

4 participants