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

Metrics name, tags & help improvements #86

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

Conversation

rturowicz
Copy link

New metrics tags added:

  • subscription id - helpful, when metrics from different subscriptions are monitored by one Prometheus instance
  • resource type - helpful, when metrics are filtered by tags

Resource name added to Target, ResourceGroup, and ResourceTag. When set, value is used as metrics name prefix (helpful when many resources from different subscriptions are monitored by one Prometheus instance).

@brian-brazil
Copy link
Member

Resource name added to Target, ResourceGroup, and ResourceTag. When set, value is used as metrics name prefix

This is an anti-pattern, see https://www.robustperception.io/target-labels-not-metric-name-prefixes

subscription id - helpful, when metrics from different subscriptions are monitored by one Prometheus instance

This should be a target label as an exporter only handles one subscription, and also it's a secret so should not be exposed as that'd be a security vulnerability.

resource type - helpful, when metrics are filtered by tags

Can you explain more about what this is, and if this information is already available in the current metrics?

@rturowicz
Copy link
Author

Resource name added to Target, ResourceGroup, and ResourceTag. When set, value is used as metrics name prefix

This is an anti-pattern, see https://www.robustperception.io/target-labels-not-metric-name-prefixes

subscription id - helpful, when metrics from different subscriptions are monitored by one Prometheus instance

This should be a target label as an exporter only handles one subscription, and also it's a secret so should not be exposed as that'd be a security vulnerability.

thanks for the advises, prefix removed in favor of custom labels

resource type - helpful, when metrics are filtered by tags

Can you explain more about what this is, and if this information is already available in the current metrics?

let's assume resource_tags config as follows:

resource_tags:
  - resource_tag_name: "monitoring"
    resource_tag_value: "enabled"
    aggregations:
      - Minimum
      - Maximum
      - Average
    metrics:
      - name: "cpu_percent"

and pg & ms sql databases with monitoring: enabled tag, then the metrics are:

cpu_percent_percent_average{resource_group="rt-main-group",resource_name="rt-test-pg"} 0.32
cpu_percent_percent_average{resource_group="rt-main-group",resource_name="rt-test-sql",sub_resource_name="rt-test-db"} 0

it's not explicitly marked which metric is for pg and which for ms sql, something like this will be more clear:

cpu_percent_percent_average{resource_group="rt-main-group",resource_name="rt-test-pg",resource_type="Microsoft.DBforPostgreSQL/servers"} 0.26
cpu_percent_percent_average{resource_group="rt-main-group",resource_name="rt-test-sql",resource_type="Microsoft.Sql/servers",sub_resource_name="rt-test-db"} 0

@brian-brazil
Copy link
Member

it's not explicitly marked which metric is for pg and which for ms sql, something like this will be more clear:

That sounds plausible, is this something that belongs on every single metric? It is technically a breaking change.

I also see you added a labels field, this has similar issues to the others you already removed from the PR.

@goya
Copy link

goya commented Aug 1, 2020

haha seems we have the same requirements! i have 3 PR's that do the same thing!

This was referenced Aug 2, 2020
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.

3 participants