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

internal/cmd: add build info metric in helm/ansible operators #4220

Merged

Conversation

joelanford
Copy link
Member

Description of the change:
Added the ansible_operator_build_info and helm_operator_build_info metrics to instrument commit and version information for the Ansible and Helm operators, respectively.

Motivation for the change:

Expose runtime version information to augment stamp versions in bundle and CSV metadata.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

I do have a slight concern about including the git commit as a metric, but I expect most operators to be using a released version of <ansible|helm>-operator so this value will likely not explode in-cluster.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2020
Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

)
)

func RegisterBuildInfo(r prometheus.Registerer) {
Copy link
Contributor

@camilamacedo86 camilamacedo86 Nov 23, 2020

Choose a reason for hiding this comment

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

@joelanford just a nit. Could you please add a comment here in this method before we get it merged?

Suggested change
func RegisterBuildInfo(r prometheus.Registerer) {
// RegisterBuildInfo ...
func RegisterBuildInfo(r prometheus.Registerer) {

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to go ahead and merge without these comments since CI already passed. If I push to this branch again, this PR will get stuck (we're currently out of Travis CI build credits).

@joelanford
Copy link
Member Author

@estroz I think the git commit hash is okay. Prometheus itself does this: https://www.robustperception.io/exposing-the-software-version-to-prometheus

@joelanford joelanford merged commit e56ad8a into operator-framework:master Nov 24, 2020
@joelanford joelanford deleted the helm-ansible-version-metric branch November 24, 2020 13:16
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
rearl-scwx pushed a commit to rearl-scwx/operator-sdk that referenced this pull request Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants