Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upscraping should not be effected when reloading Prometheus without change any config #4214
Comments
This comment has been minimized.
This comment has been minimized.
|
scrape interval become correct after this scraping |
This comment has been minimized.
This comment has been minimized.
|
this is probably where the bug is Lines 87 to 94 in bc6058c reflect.DeepEqual returned false but it should be true
|
This comment has been minimized.
This comment has been minimized.
|
are you 100% there were no config changes? |
This comment has been minimized.
This comment has been minimized.
|
I am pretty sure nobody changes the config. |
This comment has been minimized.
This comment has been minimized.
|
that is unexpected please remove any sensitive information before pasting.
|
This comment has been minimized.
This comment has been minimized.
|
the output is below
|
krasi-georgiev
added
low hanging fruit
kind/bug
component/config
labels
Jun 6, 2018
This comment has been minimized.
This comment has been minimized.
|
nice , thanks , I already see the difference in @simonpasquier , @pgier do you want to take this one? Otherwise I can have a look tomorrow. |
This comment has been minimized.
This comment has been minimized.
|
@krasi-georgiev yeah, I can take a look if you are not already working on it |
This comment has been minimized.
This comment has been minimized.
|
This appears to be the culprit. The array index of the target is set on the source field after the config has been loaded. prometheus/discovery/manager.go Lines 294 to 298 in bc6058c |
This comment has been minimized.
This comment has been minimized.
|
Thanks @pgier. Do you want to open a PR and I will test it. Should add a test to catch this. |
pgier
referenced this issue
Jun 8, 2018
Merged
config: set target group source index during unmarshalling #4245
This comment has been minimized.
This comment has been minimized.
|
Created #4245 with a fix. There are some tests already around config reloading, but I'll see if I can make a test for this specific case next week. |
This comment has been minimized.
This comment has been minimized.
|
thanks , ping when ready for review. |
This comment has been minimized.
This comment has been minimized.
|
I added a test, although its kind of very specific to this case. Feel free to review. |
pgier
added a commit
to pgier/prometheus
that referenced
this issue
Jun 12, 2018
pgier
added a commit
to pgier/prometheus
that referenced
this issue
Jun 12, 2018
brian-brazil
closed this
Jun 13, 2018
brian-brazil
added a commit
that referenced
this issue
Jun 13, 2018
krasi-georgiev
referenced this issue
Jun 13, 2018
Closed
test to ensure no change of the global config after unmarshling. #4268
brian-brazil
referenced this issue
Jun 15, 2018
Closed
Prometheus does not reload certificates on config reload #4155
brian-brazil
added a commit
that referenced
this issue
Jun 19, 2018
mknapphrt
added a commit
to mknapphrt/prometheus
that referenced
this issue
Jul 26, 2018
gouthamve
added a commit
to gouthamve/prometheus
that referenced
this issue
Aug 1, 2018
This comment has been minimized.
This comment has been minimized.
lock
bot
commented
Mar 22, 2019
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
codwu commentedJun 5, 2018
Bug Report
What did you do?
reload Prometheus without change any config
What did you expect to see?
there is no effect on target scraping
What did you see instead? Under which circumstances?

scrape interval become longer than I configured
Environment