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 common tags to Micrometer Meter Registry #644

Closed
nebhale opened this issue Oct 23, 2018 · 8 comments
Closed

Add common tags to Micrometer Meter Registry #644

nebhale opened this issue Oct 23, 2018 · 8 comments
Assignees
Milestone

Comments

@nebhale
Copy link
Member

nebhale commented Oct 23, 2018

MeterRegistry r;
r.config().meterFilter(new MySoftCommonTags());

Custom MeterFilter that examines each id and only adds new tags if they don't exist.

Also any @Bean MeterFilter gets automatically registered with the registry by Boot.

This filter should have least precedence.

Tags:

CF_APP_
if present, use these
if not present, use frigga

  • Account (default to foundation name VCAP_APPLICATION/cf_api)
  • Application (use frigga to break apart VCAP_APPLICATION/application_name)
  • Cluster (use frigga to break apart VCAP_APPLICATION/application_name)
  • Version (v001, not version guid)
  • Instance id (CF_INSTANCE_ID) cf.instance.id
  • Org/Space (best effort for org)

Use dot separators for multi-word tag names.

@nebhale nebhale added this to the v4.17 milestone Oct 23, 2018
@nebhale nebhale self-assigned this Oct 23, 2018
@nebhale nebhale removed this from the v4.17 milestone Jan 8, 2019
@nebhale nebhale assigned nebhale and unassigned nebhale Jan 8, 2019
@nebhale nebhale added this to the v4.18 milestone Feb 11, 2019
nebhale added a commit to cloudfoundry/java-buildpack-metric-writer that referenced this issue Feb 20, 2019
This change updates the dependency versions for the project.

[cloudfoundry/java-buildpack#644]

Signed-off-by: Ben Hale <bhale@pivotal.io>
nebhale added a commit to cloudfoundry/java-buildpack-metric-writer that referenced this issue Feb 20, 2019
This change adds support for a Micrometer MeterFilter that populates all
metrics with tags containing information related to the CF environment that it
is running in.  The default values, extracted from the container environment
can be overridden by environment variables.

[cloudfoundry/java-buildpack#644]

Signed-off-by: Ben Hale <bhale@pivotal.io>
@nebhale nebhale modified the milestones: v4.18, v4.19 Mar 15, 2019
@nebhale nebhale modified the milestones: v4.19, v4.20 Apr 2, 2019
@nebhale nebhale removed this from the v4.20 milestone Jul 15, 2019
@philwebb
Copy link

philwebb commented Aug 1, 2019

This Spring Boot issue discusses a similar request. I've closed that one for now so we can focus on a build-pack solution. If it turns out that the feature would be better off in Spring Boot, let me know and we can reconsider.

@nebhale
Copy link
Member Author

nebhale commented Aug 20, 2019

@tzolov and @shakuzen Can you please look over the following list and let me know if it meets your requirements and any concerns you have with it.

@snicoll As explained to me, there is no problem with the number of tags, just the cardinality of each tag as that is what drives the combinatorial explosion of time series. Most of these tags are stable for a given application with the exception of instance index and version. Instance index is typically a single digit or small double digit cardinality, and version is explicitly and emphatically not the version GUID, but rather a value derived from running frigga against the name. That should minimize the cardinality of that tag to the extent possible while still being useful.

Tag Environment Variable Default
cf.account CF_APP_ACCOUNT $VCAP_APPLICATION / cf_api
cf.application CF_APP_APPLICATION $VCAP_APPLICATION / application_name / frigga:name
cf.cluster CF_APP_CLUSTER $VCAP_APPLICATION / application_name / frigga:cluster
cf.version CF_APP_VERSION $VCAP_APPLICATION / application_name / frigga:revision
cf.instance.index CF_APP_INSTANCE_INDEX $CF_INSTANCE_INDEX
cf.organization CF_APP_ORGANIZATION $VCAP_APPLICATION / organization_name
cf.space CF_APP_SPACE $VCAP_APPLICATION / space_name

Note the tags listed here will only be added if they have not already been specified by the application in some other way.

@nebhale nebhale added this to the v4.21 milestone Aug 20, 2019
@nebhale
Copy link
Member Author

nebhale commented Aug 21, 2019

Just pushed an application called test.application-r042 and the results are what I think we're aiming for:

Adding CloudFoundry Micrometer tags if not otherwise specified: cf.account=https://api.run.pivotal.io, cf.application=test.application, cf.cluster=test.application-r042, cf.instance.index=0, cf.organization=bhale, cf.space=development, cf.version=42

nebhale added a commit that referenced this issue Aug 21, 2019
This change updates the Metric Writer to remove integration with the now EOL'd
Metrics Forwarder and instead add a set of Cloud Foundry-specific tags to
Micrometer metrics.

[resolves #644]

Signed-off-by: Ben Hale <bhale@pivotal.io>
@nebhale nebhale closed this as completed Aug 21, 2019
@bhoogter
Copy link

This change broke our metrics exporting. PCF 2.5. Spring 2.1.xx (recent). Exporting via Micrometer to Graphite. Graphite config was to append all tags/values into the path. Paths were then displayed on a Grafana dashboard.

We ended up with:

[prefix].tomcat....cf.account.VALUE.cf.application.VALUE.cf.cluster.VALUE..........Count

Took us a while to figure out if it was our code, PCF itself, some other dependency. Finally tracked it down to the build pack. The values still reported to Graphite, but to a different location due to config. This caused all the Grafana dashboards to no longer report data, as the data is now in a new location.

Solution was to lock in the buildpack version in the manifest.yml to 4.20 with:

buildpacks:
- https://github.com/cloudfoundry/java-buildpack.git#v4.20

This stopped getting the current master for java-buildpack, and got us out of this change.

It wasn't apparent how to disable this feature, as it interferes with our metrics.

@nebhale
Copy link
Member Author

nebhale commented Aug 28, 2019

@bhoogter Can you please open a separate issue with this description in it?

@shakuzen
Copy link
Contributor

shakuzen commented Sep 3, 2019

I wonder if cf.version won't be confused as referring to the version of Cloud Foundry instead of the application deployed on Cloud Foundry.

@gm2552
Copy link

gm2552 commented Sep 18, 2019

Looks this may have broken CF tasks. Getting more specific information, but seeing the following in the stack trace:

Factory method 'meterFilter' threw exception; nested exception is java.lang.IllegalStateException: Required key 'cf.instance.index' not found
 at

@nebhale
Copy link
Member Author

nebhale commented Oct 3, 2019

A heads up to everyone who requested this support (@philwebb, @tzolov, @shakuzen, and others), it appears that this change being enabled by default is causing widespread problems for customers with many different kinds of metrics monitoring (e.g. dashboards, auto-scaling, etc.).

I'm going to disable it by default but it's unclear to me, given how many people change defaults, how many people will ever enable it. It's quite possible that it'll end up being removed due to lack of usage which is quite unfortunate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants