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 Collector for Active Directory Certificate Services (ADCS) #895

Merged
merged 1 commit into from
Jan 2, 2022

Conversation

akrauza
Copy link
Contributor

@akrauza akrauza commented Dec 29, 2021

No description provided.

@akrauza akrauza requested a review from a team as a code owner December 29, 2021 06:14
Copy link
Contributor

@breed808 breed808 left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this Austin! I'm not able to test the collector myself at the moment, but I've left a few queries regarding the metric descriptions and a few other minor issues.

Let me know if any clarification is needed.

collector/adcs.go Outdated Show resolved Hide resolved
collector/adcs.go Outdated Show resolved Hide resolved
collector/adcs.go Outdated Show resolved Hide resolved
collector/adcs.go Outdated Show resolved Hide resolved
collector/adcs.go Outdated Show resolved Hide resolved
@akrauza
Copy link
Contributor Author

akrauza commented Jan 2, 2022

@breed808 Hey Ben! Thanks for the feedback on the above. Made the changes as requested. Let me know how it looks :)

Here is a screenshot of the exporter running with the above changes on a Server 2012 machine along side PerfMon:
Screen Shot 2022-01-01 at 10 13 14 PM

@akrauza akrauza requested a review from breed808 January 2, 2022 03:02
@breed808
Copy link
Contributor

breed808 commented Jan 2, 2022

Thanks Austin, I've closed all but one of the comment threads.

Once the metric names/descriptions are in order, you'll then need to update the collector documentation.

At the end we might need to squash or rebase the commits to keep the history tidy, but that's more a nitpick than an issue.

@breed808 breed808 self-assigned this Jan 2, 2022
Signed-off-by: Austin D. Krauza <[email protected]>
@akrauza
Copy link
Contributor Author

akrauza commented Jan 2, 2022

Hey @breed808 ! Thank you for your help and review. I updated the documentation and squashed the commit down into one commit message of Initial commit for ADCS collector with hash a89b5377.

Copy link
Contributor

@breed808 breed808 left a comment

Choose a reason for hiding this comment

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

Looks good!

Thanks for making those amendments Austin 👍

@breed808 breed808 merged commit 746158d into prometheus-community:master Jan 2, 2022
anubhavg-icpl pushed a commit to anubhavg-icpl/windows_exporter that referenced this pull request Sep 22, 2024
Add Collector for Active Directory Certificate Services (ADCS)
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