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

[receiver/cloudfoundry] Some metric attributes need to be moved to be resource attributes #34824

Closed
crobert-1 opened this issue Aug 22, 2024 · 11 comments

Comments

@crobert-1
Copy link
Member

Component(s)

receiver/cloudfoundry

Describe the issue you're reporting

An example metric from the Cloud Foundry receiver:

Metric #100
Descriptor:
     -> Name: system_metrics_agent.system_network_tcp_curr_estab
     -> Description: 
     -> Unit: 
     -> DataType: Gauge
NumberDataPoints #0
Data point attributes:
     -> org.cloudfoundry.index: Str(4b406f26-f1ec-4d66-8b92-f09ab6105d46)
     -> org.cloudfoundry.ip: Str(10.0.4.8)
     -> org.cloudfoundry.deployment: Str(cf-8148cfc6ff877dab3719)
     -> org.cloudfoundry.id: Str(4b406f26-f1ec-4d66-8b92-f09ab6105d46)
     -> org.cloudfoundry.job: Str(compute)
     -> org.cloudfoundry.product: Str(Small Footprint VMware Tanzu Application Service)
     -> org.cloudfoundry.instance_group: Str(compute)
     -> org.cloudfoundry.origin: Str(system_metrics_agent)
     -> org.cloudfoundry.system_domain: Str(sys.note-2482026.zf8b8cb76.shepherd.lease)
     -> org.cloudfoundry.source_id: Str(system_metrics_agent)
StartTimestamp: 2024-08-22 22:31:11.01441 +0000 UTC
Timestamp: 2024-08-22 22:31:29.45322044 +0000 UTC
Value: 51.000000

Each of these attributes are really describing the resource from where the metric and its data points are coming from, not the data point itself. These should all be moved to be resource attributes.

This would be a breaking change, and its changing the format of metrics considerably.

@crobert-1 crobert-1 added the needs triage New item requiring triage label Aug 22, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@crobert-1 crobert-1 added the bug Something isn't working label Aug 22, 2024
@crobert-1
Copy link
Member Author

@CemDK and @jriguera: Let me know if you have any feedback here.

@jriguera
Copy link
Contributor

Yes, according with OTEL guidelines, those attributes belong to resource level. The reason why they are at datapoint/log_record level is because we are copying the tags from the envelope structure. We would need to have a list of attributes which should be moved to resource level, taking into consideration there are other custom tags defined by users/operators which I would keep as datapoint/log_record attributes . There are some attributres (eg org.cloudfoundry.product which do not appear in open source Cloud Foundry)

But (just to take into consideration), one could argue that because the component is targeting CF as a system, only attributes like org.cloudfoundry.deployment , org.cloudfoundry.product and org.cloudfoundry.system_domain would be resource level attributes.

Anyway, I would prefer having them as resource level. And, yes it will be a breaking change.

@crobert-1 crobert-1 removed the needs triage New item requiring triage label Aug 23, 2024
@odubajDT
Copy link
Contributor

odubajDT commented Aug 28, 2024

Hi, I would like to look into this issue if possible

Some thoughts about the breaking change: would it be a solution to temporary (let's say for 5 releases) keep the attributes duplicated on metric level and as well on resource level? Therefore there can be a slow phasing out time for users to adapt. WDYT?

@odubajDT
Copy link
Contributor

Drafted a first possible (incomplete) draft solution, would like to hear your opinion on if duplicating the attributes is a good way in this. Thanks!

#34905

@crobert-1
Copy link
Member Author

Some thoughts about the breaking change: would it be a solution to temporary (let's say for 5 releases) keep the attributes duplicated on metric level and as well on resource level? Therefore there can be a slow phasing out time for users to adapt. WDYT?

I think a feature gate would probably be better for this scenario. Have the feature gate disabled by default, but if someone enables it, all of the relevant attributes will be resource attributes instead of (not duplicated) attributes of a datapoint.

I like the idea of documenting this though, and giving it a number of releases before upgrading the feature gate's stability.

@odubajDT
Copy link
Contributor

odubajDT commented Aug 29, 2024

Some thoughts about the breaking change: would it be a solution to temporary (let's say for 5 releases) keep the attributes duplicated on metric level and as well on resource level? Therefore there can be a slow phasing out time for users to adapt. WDYT?

I think a feature gate would probably be better for this scenario. Have the feature gate disabled by default, but if someone enables it, all of the relevant attributes will be resource attributes instead of (not duplicated) attributes of a datapoint.

I like the idea of documenting this though, and giving it a number of releases before upgrading the feature gate's stability.

Thanks for the response, adapted the implementation and should be ready for review now!

@jriguera
Copy link
Contributor

I still need to review the code, but I agree with the feature gate.

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Oct 29, 2024
dmitryax pushed a commit that referenced this issue Dec 20, 2024
…level (#34905)

**Description:** <Describe what has changed.>
Introduce a feature gate enable copying envelope tags to the metrics as
resource attributes instead of datapoint attributes.

**Link to tracking Issue:** #34824

---------

Signed-off-by: odubajDT <[email protected]>
Copy link
Contributor

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 28, 2024
@crobert-1 crobert-1 reopened this Jan 6, 2025
@crobert-1
Copy link
Member Author

Resolved by #34905

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

3 participants