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: Allow filtering metric attributes and creating custom monitored resource #850

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bhshkh
Copy link

@bhshkh bhshkh commented May 8, 2024

Context: https://github.com/googleapis/google-cloud-go/pull/10046/files#diff-a1b4b02d8e5fa0e59a2122e76a7bcda69fdd818d66ee4498db5b8d8100ec9cabR45-R61

I'm working on exporting Bigtable client metrics to Google Cloud Monitoring.
The metrics need to be written with monitored resource type 'bigtable_client_raw'
The monitored resource needs to have below labels:

  • project_id
  • instance
  • table
  • cluster
  • zone

The values for the last 3 of the above labels need to be obtained from response metadata i.e. they are not available at the time of otel resource creation i.e. unavailable at this point in code:

import "go.opentelemetry.io/otel/sdk/resource"

res, err := resource.New(ctx)
if err != nil {
	return err
}

How to include these in monitored resource labels?

  1. While recording the metrics, add these labels to the metrics
  2. Exporter will use the function provided in the WithMonitoredResourceCreator to extract the resource labels from the metric labels and create monitored resource. Example usage:

https://github.com/googleapis/google-cloud-go/pull/10046/files#diff-a1b4b02d8e5fa0e59a2122e76a7bcda69fdd818d66ee4498db5b8d8100ec9cabR58-R59

https://github.com/googleapis/google-cloud-go/pull/10046/files#diff-a1b4b02d8e5fa0e59a2122e76a7bcda69fdd818d66ee4498db5b8d8100ec9cabR71-R112

Also, the resource labels should be removed from the metric labels before exporting them to Google Cloud Monitoring. This will be done using the WithFilteredMetricAttributes option. Example usage:

https://github.com/googleapis/google-cloud-go/pull/10046/files#diff-a1b4b02d8e5fa0e59a2122e76a7bcda69fdd818d66ee4498db5b8d8100ec9cabR61

Copy link

codecov bot commented May 8, 2024

Codecov Report

Attention: Patch coverage is 37.50000% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 62.58%. Comparing base (4caace7) to head (be038e8).
Report is 17 commits behind head on main.

Files Patch % Lines
exporter/metric/option.go 0.00% 6 Missing ⚠️
exporter/metric/metric.go 60.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #850      +/-   ##
==========================================
+ Coverage   61.03%   62.58%   +1.54%     
==========================================
  Files          56       57       +1     
  Lines        5903     4931     -972     
==========================================
- Hits         3603     3086     -517     
+ Misses       2143     1685     -458     
- Partials      157      160       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bhshkh bhshkh changed the title feat: Allow filtering resources and creating custom monitored resource feat: Allow filtering metric attributes and creating custom monitored resource May 8, 2024

// metricAttributesFilter determinies which metric attributes to
// add to metrics as metric labels
metricAttributesFilter attribute.Filter
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need this config in the exporter. Use https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric#WithView to pass views to the metrics SDK, which allow filtering attributes on metrics.

@dashpole
Copy link
Contributor

dashpole commented May 8, 2024

cc @psx95

@psx95
Copy link
Contributor

psx95 commented May 9, 2024

cc @psx95

This looks similar to the recent request we had for the Java exporter, I will take a look into this PR.

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.

None yet

3 participants