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

[pkg/ottl] Provide Context at Metric Level instead of DataPoint Level #13838

Closed
TylerHelmuth opened this issue Sep 1, 2022 · 6 comments
Closed
Labels
enhancement New feature or request pkg/ottl priority:p2 Medium

Comments

@TylerHelmuth
Copy link
Member

TylerHelmuth commented Sep 1, 2022

Is your feature request related to a problem? Please describe.
The current tqlmetrics context is based on GetItem() returning a datapoint. This is useful for interacting with datapoints, but makes certain transformations, such as metrics aggregation impossible.

Describe the solution you'd like
A context at the metrics level that will have access to all the datapoints of a metric.

Additional context
The new context will, I think, struggle to create Getters for individual datapoint values. For example, for a condition like ... where data_points.attributes["test"] == "pass", how will the context provide a Getter that is the values of all the datapoints attribute named "test"?

Setters will be ok, because the context can be smart enough to iterate through each data point, but it will not be conditional to the individual data point; it would be all or nothing.

Example implementation (for only Sum):

func accessAttributesKey(mapKey *string) tql.StandardGetSetter {
	return tql.StandardGetSetter{
		Getter: func(ctx tql.TransformContext) interface{} {
			// how do we return a Getter that is the value of an attribute of each data point?
			return nil
		},
		Setter: func(ctx tql.TransformContext, val interface{}) {
			metric := ctx.(TransformContext).GetItem().(pmetric.Metric)
			switch metric.DataType() {
			case pmetric.MetricDataTypeSum:
				for i := 0; i < metric.Sum().DataPoints().Len(); i++ {
					tqlcommon.SetMapValue(metric.Sum().DataPoints().At(i).Attributes(), *mapKey, val)
				}
			}
		},
	}
}

It is possible this limitation is acceptable; if you need to interact with individual data points then use the datapoint context. It might be inappropriate for the metrics context to provide access to anything lower than the DataPoints slice.

@TylerHelmuth TylerHelmuth added enhancement New feature or request priority:p2 Medium pkg/ottl labels Sep 1, 2022
@TylerHelmuth
Copy link
Member Author

@dmitryax I am curious if any of the metricstransform processor scenarios require individual datapoint interactions?

@dmitryax
Copy link
Member

dmitryax commented Sep 7, 2022

@dmitryax I am curious if any of the metricstransform processor scenarios require individual datapoint interactions?

Yes, things like attribute renaming, dropping datapoints that match attributes

@TylerHelmuth
Copy link
Member Author

@dmitryax could those be handled by a "datapoint" context tho? Or are you saying that while interacting at the "metric" level we also need to be able to mess with individual data points?

@dmitryax
Copy link
Member

dmitryax commented Sep 7, 2022

Or are you saying that while interacting at the "metric" level we also need to be able to mess with individual data points?

This is what's happening in metrics transform. We don't have concept of context there.

@TylerHelmuth
Copy link
Member Author

I'll need to keep iterating on how to handle this then. If we need to be able to interact with an individual DataPoint at the same time (with in the same function/group of functions) that we interact with the metric/DataPointsSlice then we'll need to do some rearchitecting.

@TylerHelmuth
Copy link
Member Author

@dmitryax @bogdandrutu looking through the metricstransformprocessor's current functionality, I think it can be broken down into these 2 separate OTTL Contexts

Operation Example (based on metric system.cpu.usage) OTTL Context
Rename metrics Rename to system.cpu.usage_time Metric
Add labels Add new label identifier with value 1 to all points Metric or* DataPoint
Rename label keys Rename label state to cpu_state Metric or* DataPoint
Rename label values For label state, rename value idle to - Metric or* DataPoint
Delete data points Delete all points where label state has value idle Metric
Toggle data type Change from int data points to double data points Metric or* DataPoint
Scale value Multiply values by 1000 to convert from seconds to milliseconds Metric or* DataPoint
Aggregate across label sets Retain only the label state, average all points with the same value for this label Metric
Aggregate across label values For label state, sum points where the value is user or system into used = user + system Metric

* use Metric if need to affect all DataPoints in a metric and use DataPoint if only want to change a subset of DataPoints.

Reading through the readme I don't see any operations that both aggregate and interact with specific data points. For this reason I think it would be possible to transition all metricstransformprocessor functionality to a processor using OTTL if we provide both a Metric and DataPoint context.

In the transformprocessor, we'll need a way to specify the order of operations between statements that using Metric and statements that use DataPoint, but that seems doable within the configuration file.

@TylerHelmuth TylerHelmuth changed the title [pkg/telemetryquerylanguage] Provide Context at Metric Level instead of DataPoint Level [pkg/ottl] Provide Context at Metric Level instead of DataPoint Level Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg/ottl priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

2 participants