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

mdatagen support for slice and map attributes #18272

Closed
Tracked by #10904
fredthomsen opened this issue Feb 3, 2023 · 12 comments
Closed
Tracked by #10904

mdatagen support for slice and map attributes #18272

fredthomsen opened this issue Feb 3, 2023 · 12 comments
Labels
cmd/mdatagen mdatagen command enhancement New feature or request help wanted Extra attention is needed

Comments

@fredthomsen
Copy link
Contributor

Component(s)

cmd/mdatagen

Is your feature request related to a problem? Please describe.

I would like for mdatagen to support generation of code supporting attributes that are maps or slices. I am attempting to add a resource attribute which is a list of strings containing some information that should be acted on by a downstream processor (I realize that #16811 makes this unfeasible at the moment).

Describe the solution you'd like

In the metadata.yml file used by mdatagen, I would like to be able to use a configuration like the following:

resource_attributes:
  sl.app.classifications:
    description: A list of classifications for this data
    type: strings

And this would generate a function like this for setting the resource attribute:

func WithSlAppClassifications(vals []string) ResourceMetricsOption {
       return func(rm pmetric.ResourceMetrics) {
               c := rm.Resource().Attributes().PutEmptySlice("sl.app.classifications")
               for _, val := range vals {
                       c.AppendEmpty().SetStr(val)
        }
 }

Similar approach should be taken for metric attributes by providing their RecordXDataPoint functions.

Describe alternatives you've considered

Just use a attribute string that is split by a known deliminator such as |.

Additional context

The above just assumes a single-level of nesting. With multiple levels specifying the types at each level requires a different way to specify the type.

@fredthomsen fredthomsen added enhancement New feature or request needs triage New item requiring triage labels Feb 3, 2023
@github-actions github-actions bot added the cmd/mdatagen mdatagen command label Feb 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2023

Pinging code owners:

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

@dmitryax
Copy link
Member

dmitryax commented Feb 3, 2023

Thanks for reporting this @fredthomsen. Which receiver do you want to use this functionality in?

@fredthomsen
Copy link
Contributor Author

We have a custom receiver that we have written to scrape infrastructure data from our monitoring system (this is still pretty alpha) and we were planning on using these resource tags to make some downstream storage routing decisions. Also, I am happy to handle this work if you agree this functionality is desirable.

@dmitryax
Copy link
Member

dmitryax commented Feb 3, 2023

Yes, I think this functionality is desirable. Fill free to submit a PR

@dmitryax
Copy link
Member

dmitryax commented Feb 3, 2023

Couple of comments:

Let's stick to go type definitions in metadata.yaml:

resource_attributes:
  sl.app.classifications:
    description: A list of classifications for this data
    type: []string

I believe we can make the generated code simple and have the following:

func WithSlAppClassifications(vals []string) ResourceMetricsOption {
       return func(rm pmetric.ResourceMetrics) {
               rm.Resource().Attributes().FromRaw(vals)
        }
 }

Setting å resource attribute is one operation per scrape, so it's not a performance critical part

@fredthomsen
Copy link
Contributor Author

Setting å resource attribute is one operation per scrape, so it's not a performance critical part

I used resource attributes as an example, but was planning on doing this for metric attributes as well.

@dmitryax
Copy link
Member

dmitryax commented Feb 3, 2023

FromRaw for maps and slices should be as performant as your example. Let's use it and keep assignments for other types as is for now

@fredthomsen
Copy link
Contributor Author

@dmitryax PR is out, but I am having to deal with some conversions from []string to []any and map[string]string to map[string]any to leverage FromRaw. I am sure those implications are worth discussing.

@Aneurysm9
Copy link
Member

I used resource attributes as an example, but was planning on doing this for metric attributes as well.

Note that maps and heterogeneous arrays are not valid attribute values for metrics or resource attributes. This may change in the (maybe near) future, but I'm not sure we should get out ahead of the spec here.

/cc @open-telemetry/collector-approvers

@dmitryax
Copy link
Member

@Aneurysm9 Thanks for pointing to the PR. Looks like it's close to being merged. Given that it's supported in the OTLP, I think we should be good to add the mdatagen support once the spec PR is merged.

@github-actions
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 Apr 11, 2023
@mx-psi mx-psi removed the Stale label Apr 11, 2023
fredthomsen added a commit to fredthomsen/opentelemetry-collector-contrib that referenced this issue Apr 28, 2023
Add support for map and slice resource and datapoints attributes in
order to fulfill open-telemetry#18272.
dmitryax pushed a commit that referenced this issue Apr 29, 2023
Add support for map and slice attributes

Add support for map and slice resource and datapoints attributes in
order to fulfill #18272.
@dmitryax
Copy link
Member

Resolved by #18487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/mdatagen mdatagen command enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants