Skip to content

Conversation

@SaveTheRbtz
Copy link
Contributor

This diff adds an ability to pass custom Context to the collector. This is inline with some other exporter's like the mysqld_exporter.

SaveTheRbtz referenced this pull request Mar 25, 2021
Make it easier to embed the snmp_exporter into other systems.
* Update metric registration to use promauto.
* Update minimum version to Go 1.14. (required for exporter-toolkit)
* Update snmp.yml with latest mibs.

Re-implementation of #456

Signed-off-by: Ben Kochie <[email protected]>
@SuperQ
Copy link
Member

SuperQ commented Mar 26, 2021

Ahh, yes, it would be useful use the X-Prometheus-Scrape-Timeout-Seconds header.

I'm not sure how useful it is to stub this out, without implementing any use of the context is. Are you planning to add more to this PR?

@SaveTheRbtz
Copy link
Contributor Author

We use collector directly as a library so we pass our own context therefore I marked the main.go context as TODO. I you want I can add support for X-Prometheus-Scrape-Timeout-Seconds in this diff, or a subsequent one.

@SuperQ
Copy link
Member

SuperQ commented Mar 28, 2021

Yes, with normal use, no context is necessary, as the scrape drops the HTTP connection on timeout, which cancels the walk. I was wrong, we don't need to explicitly handle the X-Prometheus-Scrape-Timeout-Seconds here. It's automatic.

Signed-off-by: Alexey Ivanov <[email protected]>
@SaveTheRbtz
Copy link
Contributor Author

Ok. I've added context propagation from the original HTTP request to the underlying collector. This should ensure cancelation in following scenarios:

For incoming server requests, the context is canceled when the client's connection closes, the request is canceled (with HTTP/2), or when the ServeHTTP method returns.

@SuperQ
Copy link
Member

SuperQ commented Mar 28, 2021

Thanks, it looks like I screwed up the context here. This puts it back to where it was.

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM.

@SuperQ SuperQ merged commit 1592a88 into prometheus:main Mar 29, 2021
SuperQ added a commit that referenced this pull request Nov 21, 2022
* [CHANGE] Update to exporter-toolkit v0.8.1 (#810)
* [FEATURE] Support chained lookups in the generator (#757)
* [ENHANCEMENT] Add per-SNMP packet statistics. (#656)
* [ENHANCEMENT] Add support for aes192c and aes256c privacy protocol (#657)
* [ENHANCEMENT] Support responding from different source address (#702)
* [BUGFIX] Fixes dropped context passing (#634)
* [BUGFIX] Add version flag (#717)
* [BUGFIX] Fix retries in generator (#786)

Signed-off-by: SuperQ <[email protected]>
@SuperQ SuperQ mentioned this pull request Nov 21, 2022
SuperQ added a commit that referenced this pull request Nov 21, 2022
* [CHANGE] Update to exporter-toolkit v0.8.1 (#810)
* [FEATURE] Support chained lookups in the generator (#757)
* [ENHANCEMENT] Add per-SNMP packet statistics. (#656)
* [ENHANCEMENT] Add support for aes192c and aes256c privacy protocol (#657)
* [ENHANCEMENT] Support responding from different source address (#702)
* [BUGFIX] Fixes dropped context passing (#634)
* [BUGFIX] Add version flag (#717)
* [BUGFIX] Fix retries in generator (#786)

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this pull request Nov 22, 2022
* [CHANGE] Update to exporter-toolkit v0.8.1 (#810)
* [FEATURE] Support chained lookups in the generator (#757)
* [ENHANCEMENT] Add per-SNMP packet statistics. (#656)
* [ENHANCEMENT] Add support for aes192c and aes256c privacy protocol (#657)
* [ENHANCEMENT] Support responding from different source address (#702)
* [BUGFIX] Fixes dropped context passing (#634)
* [BUGFIX] Add version flag (#717)
* [BUGFIX] Fix retries in generator (#786)

Signed-off-by: SuperQ <[email protected]>

Signed-off-by: SuperQ <[email protected]>
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.

2 participants