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

Fix documented default value for community #1084

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

PierreF
Copy link
Contributor

@PierreF PierreF commented Jan 2, 2024

The documentation (README) say that the default value for the SNMP community is "public_v2". But I believe this is wrong:

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.

The default is actually implemented in main.go. The struct default is ignored since it's actually part of the URL parm parsing.

If you curl the exporter with no auth params, you can see it in the debug log.

$ curl 'http://localhost:9116/snmp?target=192.168.1.1'

Result:

ts=2024-01-11T11:33:33.332Z caller=collector.go:464 level=debug auth=public_v2 target=192.168.1.1 module=if_mib msg="Finished scrape" duration_seconds=5.002192868

If you don't mind, please update the DefaultAuth value in config/config.go to match the new default.

@PierreF
Copy link
Contributor Author

PierreF commented Jan 11, 2024

In main.go I see the default for the auth which default to "public_v2" which I'm fine with that.

In this PR I'm talking about the SNMP community send in the SNMP network query which currently had default to "public" but it's documented as "public_v2". And hopefully the default is "public" or I think it would break collection on most SNMP device which respond to "public" community.

So I'm not sure what you want:

  • I don't want to change the default SNMP community from "public" to "public_v2" to match current documentation (due to risk breaking collection)
  • I'm not sure changing the default auth from "public_v2" to "public" is a good idea, as it could cause breaking change on API

@SuperQ
Copy link
Member

SuperQ commented Jan 11, 2024

Oh, right, I'm confusing which strings mean which things.

generator/README.md Outdated Show resolved Hide resolved
@PierreF PierreF force-pushed the fix-typo-in-generator-doc branch from 45b9625 to 057a0f7 Compare January 11, 2024 13:12
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.

Thanks!

@SuperQ SuperQ merged commit 8b06bcb into prometheus:main Jan 11, 2024
6 checks passed
harshavmb pushed a commit to harshavmb/snmp_exporter that referenced this pull request Feb 16, 2024
Signed-off-by: Pierre Fersing <[email protected]>
Signed-off-by: Harshavardhan Musanalli <[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