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

Add type to parse date and time #1234

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Conversation

phibos
Copy link
Contributor

@phibos phibos commented Aug 13, 2024

I have added a new type ParseDateAndTime to parse timestamps from DisplayString and report it as unix timestamp.

Please have a look and report if something is missing.

This fixes #1232

For now it uses the go time.Parse() function. If we want to swap it out there are some packages.

What do you think?

@phibos phibos force-pushed the parse_date_and_time branch from 1b80bde to 0740933 Compare August 13, 2024 11:54
@SuperQ
Copy link
Member

SuperQ commented Aug 16, 2024

Nice, I like it so far. I haven't had a chance to evaluate the parser libraries.

@SuperQ
Copy link
Member

SuperQ commented Aug 18, 2024

I personally prefer strftime style parsing. I sent a couple of PRs to https://github.com/lestrrat-go/strftime to see if the maintainer is still active.

What do you think @bastischubert? Do you prefer Go or strftime style date parsing syntax?

@bastischubert
Copy link
Member

I'm not in favor of any specific parsing syntax - whatever works and doesn't introduce unnecessary complexity if ok (-:

@phibos
Copy link
Contributor Author

phibos commented Aug 27, 2024

@SuperQ did you find the time to have a look at the time parser libraries? Do I have to change something or can this PR be merged?

@SuperQ
Copy link
Member

SuperQ commented Aug 27, 2024

I looked over https://github.com/lestrrat-go/strftime, it seems reasonable. I'm fine with either library, but I live a slight preference for strftime.

@phibos
Copy link
Contributor Author

phibos commented Aug 27, 2024

Thanks for you reply. I will update the PR and use the strftime functions to parse the date and time. But before I change the code. Do you see any reason why we should provide an config option to specify the format type?

Example:

datetime_format_type: go | strftime

Add new type ParseDateAndTime to parse timestamps from DisplayString
and report it as unix timestamp.

Signed-off-by: PhiBo <[email protected]>
@phibos phibos force-pushed the parse_date_and_time branch from 0740933 to 73e4c33 Compare August 27, 2024 13:00
@SuperQ
Copy link
Member

SuperQ commented Aug 27, 2024

Adding multiple parser options is probably a bit more complexity than necessary. I'd like to start with one parser and see how things evolve.

@phibos phibos force-pushed the parse_date_and_time branch from 80ec12c to e0b09de Compare August 29, 2024 15:30
@phibos
Copy link
Contributor Author

phibos commented Aug 29, 2024

Had to use https://github.com/itchyny/timefmt-go instead of https://github.com/lestrrat-go/strftime because it does not support parsing lestrrat-go/strftime#22

@phibos phibos force-pushed the parse_date_and_time branch from e0b09de to 1b40661 Compare August 29, 2024 16:07
@SuperQ SuperQ merged commit 1d75633 into prometheus:main Aug 30, 2024
6 checks passed
@phibos phibos deleted the parse_date_and_time branch August 30, 2024 12:05
SuperQ added a commit that referenced this pull request Jan 3, 2025
BREAKING CHANGES:

This version of the exporter introduces a cleaned up default snmp.yml that moved all
ucd-snmp-mib oids into a separate module.

If you used one of the following modules:
* synology
* ddwrt
* kemp_loadmaster 

you will need to change your scrape config to also include the ucd_la_table module as well.
See https://github.com/prometheus/snmp_exporter/tree/main?tab=readme-ov-file#multi-module-handling for further instructions.

* [CHANGE] generator: Update generator default MIBOPTS #1231
* [CHANGE] adopt log/slog, drop go-kit/log #1249
* [ENHANCEMENT] generator: Improve config error message #1274
* [FEATURE] add ParseDateAndTime type #1234 
* [FEATURE] Set UseUnconnectedUDPSocket option if one of the modules has if set #1247
* [FEATURE] add NTPTimeStamp type #1315
* [BUGFIX] fixed dashboard mixins #1319

snmp.yml changes:
* cleanup ucd-snmp-mibs #1200
  * moved oids from synology,ddwrt and kemp_loadmaster to new module ucd_la_table 
* Added support for Sophos XG Series #1239
* Added support for HPE #1267
* Added support for powercom #1275
* Added support for Cisco IMC #1293
* Updated mib for apc #1303
* Added support for TPLink DDM #1304

---------

Signed-off-by: Sebastian Schubert <[email protected]>
Signed-off-by: Sebastian Schubert <[email protected]>
Co-authored-by: Ben Kochie <[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.

Parse date and time from DisplayString
3 participants