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

feat: POC of adding subscription_name to metric labels #2279

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jiping-s
Copy link

@jiping-s jiping-s commented Apr 13, 2023

This is a proof of concept for adding subscription_name (1:1 to subscription_id) for metrics. I don't have time to complete it for all the requirements, but if anyone is interested please take it over.

Applies to both of azure metrics and promitor's internal metrics.

  • Verified manually with Prometheus sink; No other sink tested and no unit tests
  • Works for resources discovered by ResourceDiscovery only

How it works:

  • A new API endpoint in ResourceDiscovery to expose list of subscription ID -> name
  • The API is queried in scraper in the same schedule jobs for azure metrics. ID->Name is only added or updated, never deleted.
  • In scraper, AzureScrapingSystemMetricsPublisher adds associated subscription_name if there is a subscription_id label <- for promitor's internal metrics
  • In scraper, MetricSinkWriter adds associated subscription_name if there is a subscription_id label <- for azure metrics

@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Apr 13, 2023
@CLAassistant
Copy link

CLAassistant commented Apr 13, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ tomkerkhove
❌ jiping-s
You have signed the CLA already but the status is still pending? Let us recheck it.

@tomkerkhove
Copy link
Owner

Thank you! I'll review it asap

@tomkerkhove
Copy link
Owner

This is for #1665 I presume?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔍 Ready for Review Pull Request is not reviewed yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants