-
Notifications
You must be signed in to change notification settings - Fork 271
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
Upgrade to prometheus 2.55 #2868
Conversation
c667866
to
e7ed631
Compare
💻 Deploy preview deleted. |
@@ -71,6 +74,8 @@ type Arguments struct { | |||
ScrapeClassicHistograms bool `alloy:"scrape_classic_histograms,attr,optional"` | |||
// Whether to scrape native histograms. | |||
ScrapeNativeHistograms bool `alloy:"scrape_native_histograms,attr,optional"` | |||
// File to which scrape failures are logged. | |||
ScrapeFailureLogFile string `alloy:"scrape_failure_log_file,attr,optional"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can make this more generic, before we commit to this. Looking at the callback this is the filename in the NewManager callback, could we override that with a new entity that would allow use to have a generic output and then have a local.output.file
, output to logs, and other outputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed offline, we could polish this further at a later date. Given that it's a feature for debugging, breaking changes to it would be fine. My personal opinion is that:
- It'd be best to integrate this with the debug logs of the component. But I haven't done it yet since I'm not sure if it'd spam too much, or if there's some other reason for having a separate file that I could be missing.
- I'd rather not add this feature to live debugging since live debugging is for seeing the data flowing through a component. It's good to have a clear separation between this and the component's own telemetry.
- I'd prefer not to have a complex way to send this log to other components... it's just part of the component's own logs and we should try treat it the same way as its regular logs.
Ideally we should have the ability to enable debug logging for a specific component instance, and maybe even set what sort of features in a component should be doing debug logging. Then we can integrate such debug logs very easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable, but want to take a second to talk about scrape failure log before committing to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving for docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Add an http headers arg * Add a scrape_failure_log_file argument * Update go mod * Add `http_headers` to doc * Validate HTTP headers
4fab89c
to
320de87
Compare
PR Description
This is an incremental step towards an upgrade to Prometheus 3.
Notes to the Reviewer
PR Checklist