Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add build_info metric and use it in generated queries #35

Merged
merged 26 commits into from
May 11, 2023
Merged

Conversation

brettimus
Copy link
Collaborator

@brettimus brettimus commented May 10, 2023

Implements #25


Description for Reviewers

We should discuss how we want to pull in the commit and version info, especially in relation to the expected behavior in VSCode.

What Changed

  • Add an instance method of set_build_info to both the Prometheus and Otel trackers

    • Make sure that set_build_info is only called once (per tracker instance)
    • Note that the OTEL tracker implements build_info as an UpDownCounter.
  • When create_tracker is called, we will automatically call set_build_info with the AUTOMETRICS_COMMIT and AUTOMETRICS_VERSION environment variables

  • Update PromQL queries to use commit and version info

TODO

  • Update README regarding the environment variables we want to support for version/commit
  • Check whether OTEL tracker should also use the name build_info
  • Only call tracker set_build_info once

@brettimus brettimus self-assigned this May 10, 2023
@brettimus
Copy link
Collaborator Author

Update: OTEL support will be dependent on resolution of open-telemetry/opentelemetry-python#3071 (I'm working on a PR)

@brettimus
Copy link
Collaborator Author

brettimus commented May 10, 2023

I think we can treat the OTEL support as non-blocking, since I'm not sure how long it will take for my PR on opentelemetry-python to get reviewed/merged/released

open-telemetry/opentelemetry-python#3306

@brettimus brettimus marked this pull request as ready for review May 10, 2023 16:31
Copy link
Member

@flenter flenter left a comment

Choose a reason for hiding this comment

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

🎉 Nice!

However, I think it would be good to expand the TrackerType protocol (i.e. interaface) with a set_build_info function (defined in src/autometrics/tracker/tracker.py)

@@ -3,6 +3,8 @@
from typing import Optional
from dotenv import load_dotenv

ADD_BUILD_INFO_LABELS = "* on (instance, job) group_left(version, commit) build_info"
Copy link
Member

Choose a reason for hiding this comment

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

The grafana dashboards use a slightly different query:

* on(instance, job) group_left(version, commit) (
    last_over_time(
      build_info[1s]
    ) or on (instance, job) up
  )

Perhaps we should do this here as well? Here's the PR with the related change on the autometrics-shared repo: autometrics-dev/autometrics-shared#8

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah good catch! i updated the queries + tests

Copy link
Member

@flenter flenter left a comment

Choose a reason for hiding this comment

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

2 minor things, but in general LGTM! 👍

@brettimus brettimus merged commit 1f05706 into main May 11, 2023
@brettimus brettimus deleted the add-build-info branch May 11, 2023 12:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants