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

[processor/transform] Add the capability to scale a metric value #16214

Closed
Tracked by #16223
gabrielgiussi opened this issue Nov 9, 2022 · 12 comments
Closed
Tracked by #16223

[processor/transform] Add the capability to scale a metric value #16214

gabrielgiussi opened this issue Nov 9, 2022 · 12 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers never stale Issues marked with this label will be never staled and automatically removed priority:p2 Medium processor/transform Transform processor

Comments

@gabrielgiussi
Copy link

Component(s)

processor/transform

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

Currently the metricstransformer already supports scaling a value and a recent PR even extended this for histograms.

Since processor/transformer will eventually replace metricstransformer could we add this feature?

If you agree with feature request I could work on this.

Describe the solution you'd like

Scale a value down/up, even histograms, e.g. to transform a histogram in milliseconds to seconds

scale(0.001) where metric.name == "envoy_vhost_vcluster_upstream_rq_time"

Describe alternatives you've considered

No response

Additional context

No response

@gabrielgiussi gabrielgiussi added enhancement New feature or request needs triage New item requiring triage labels Nov 9, 2022
@github-actions github-actions bot added the processor/transform Transform processor label Nov 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022

Pinging code owners:

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

@TylerHelmuth TylerHelmuth added priority:p2 Medium help wanted Extra attention is needed and removed needs triage New item requiring triage labels Nov 9, 2022
@TylerHelmuth
Copy link
Member

A solution for this may have some overlap with #16067

@TylerHelmuth TylerHelmuth removed the help wanted Extra attention is needed label Nov 9, 2022
@TylerHelmuth
Copy link
Member

TylerHelmuth commented Nov 9, 2022

@gabrielgiussi yes the transform processor should be able to handle this. I think we can make a generic function that takes a float or int and scales it based on a factor. Instead of scale setting the value itself, though, I think it would work better as a Factory Function (a function that modifies data and then returns it) would be best so that the result can be used anywhere.

Function signature could look like Scale(getter, factor). The we could do things like

  • set(value_double, Scale(value_double, 10))
  • set(attributes["my-number"], Scale(value_double, 100))
  • set(start_time_unix_nano, Scale(start_time_unix_nano, .0001) <- I'm really interested in this implication. I think it would screw with stuff bc the start_time_unix_nano setter expects a nano time.

@TylerHelmuth TylerHelmuth changed the title Add the capability to scale a metric value in processor/transform [processor/transform] Add the capability to scale a metric value Nov 16, 2022
@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 Jan 16, 2023
@github-actions
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 May 27, 2023
@TylerHelmuth TylerHelmuth added never stale Issues marked with this label will be never staled and automatically removed and removed Stale closed as inactive labels May 30, 2023
@TylerHelmuth TylerHelmuth reopened this May 30, 2023
@TylerHelmuth
Copy link
Member

@gabrielgiussi OTTL supports arithmetic operations which should handle this use-case:

set(value_double, value_double*0.001) where metric.name == "envoy_vhost_vcluster_upstream_rq_time"

I do think there is a limitation with scaling histograms.

@bacherfl
Copy link
Contributor

If this ticket is available, I would like to work on this @evan-bradley FYI

@bacherfl
Copy link
Contributor

@TylerHelmuth @evan-bradley just a quick question for clarification - Since arithmetic operations can be used for achieving this, as suggested here, is the goal now to either:

  • have the arithmetic operations also support handling Histograms, or to
  • implement the Scale factory function that can handle both scalar values and histograms?

@TylerHelmuth
Copy link
Member

@bacherfl thanks for looking at this.

I am open to opinions on whether we just need histogram support or a new function.

I believe a new function would provide the most consistent workflow for end users, and it could also optionally allow updating the metric unit.

@TylerHelmuth TylerHelmuth removed the help wanted Extra attention is needed label May 28, 2024
@evan-bradley
Copy link
Contributor

I would also agree we should implement a function.

I would be in favor or either a scale_metric Editor that only operates on all data points within a metric, or a Scale Converter that works on both whole metrics and on individual data points. The Converter would probably be a little more verbose in general usage (set(data_points, Scale(data_points, 10))), but would allow for things like scaling a misadjusted data point if that's ever an issue.

A Scale converter could be good for adjusting individual integer/float values, but I think relying on arithmetic for that is okay.

@TylerHelmuth
Copy link
Member

Operating on all the datapoints of a metric is the most similar to the metricstransformprocessor

@bacherfl
Copy link
Contributor

Thanks for your replies @TylerHelmuth @evan-bradley - I implemented this as a new converter function now - feel free to check out the linked PR :)

TylerHelmuth added a commit that referenced this issue Jul 25, 2024
**Description:**

Adds a `Scale` function to the OTTL package. This function can be
applied to `int`/`double` values, as well as metrics of the following
types:

- Sum
- Gauge
- Histogram

**Link to tracking Issue:** #16214

**Testing:** Added Unit and E2E tests in the OTTL package. Tested
manually in a sample environment with the following example
configuration:

```
receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317

processors:
  transform:
    error_mode: ignore
    metric_statements:
      - context: metric
        statements:
          - set(data_points, Scale(data_points, 10.0))

exporters:
  debug:
    verbosity: detailed
  otlphttp:
    endpoint: "######"
    headers:
      Authorization: "#####"

service:
  pipelines:
    metrics:
      receivers: [otlp]
      exporters: [otlphttp, debug]
      processors: [transform]
```

**Documentation:** Added documentation in the `README` describing all
functions in the `ottl` package

---------

Signed-off-by: Florian Bacher <[email protected]>
Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Tyler Helmuth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers never stale Issues marked with this label will be never staled and automatically removed priority:p2 Medium processor/transform Transform processor
Projects
None yet
Development

No branches or pull requests

4 participants