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

Configurable scrape_interval is broken, breaks 1m Grafana graphs #34

Closed
pforman opened this issue Jul 19, 2017 · 2 comments · Fixed by #35
Closed

Configurable scrape_interval is broken, breaks 1m Grafana graphs #34

pforman opened this issue Jul 19, 2017 · 2 comments · Fixed by #35
Assignees

Comments

@pforman
Copy link

pforman commented Jul 19, 2017

The changes in #33 made for a surprise, when the success rate and request volume graphs became empty.

The default of 1m matches the query in Grafana, so the graphs become empty when they don't have two data points. I'd recommend a different default, like 30s.

However, this can't be overridden, because the replacement with $SCRAPE_INTERVAL fails.

The root of the issue is here:

sed -i "" "s@scrape_interval:.*@scrape_interval: $SCRAPE_INTERVAL@" $PROMETHEUS_CONF
sed -i "" "s@ evaluation_interval:.*@ evaluation_interval: $SCRAPE_INTERVAL@" $PROMETHEUS_CONF

With sed -i "" , the "" is interpreted as a filename by GNU sed, and the sed command fails. So the replacements never happen, and the command is only ever executed with the default "1m" in prometheus-$PLATFORM.yml . (whichever file that turns out to be, "k8s" in my case).

This should be sed -i"", with no space. (verified inside the container)

It would be a two-character PR, but the default causing empty graphs is also surprise, so the defaults should probably be adjusted down to ensure the irate() call has data.

@wmorgan
Copy link
Member

wmorgan commented Jul 19, 2017

@pforman thanks for the detailed analysis! We will take a look.

@siggy
Copy link
Member

siggy commented Jul 19, 2017

@pforman Thanks for the report! Fix is up at #35.

@siggy siggy closed this as completed in #35 Jul 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants