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

Allowing Remap to manipulate Metric values #5521

Open
StephenWakely opened this issue Dec 12, 2020 · 11 comments
Open

Allowing Remap to manipulate Metric values #5521

StephenWakely opened this issue Dec 12, 2020 · 11 comments
Labels
meta: idea Anything in the idea phase. Needs further discussion and consensus before work can begin. needs: approval Needs review & approval before work can begin. transform: remap Anything `remap` transform related type: enhancement A value-adding code change that enhances its existing functionality.

Comments

@StephenWakely
Copy link
Contributor

Referencing the further work in RFC 3550 - 2020-11-01

As per the RFC ond #5475 Remap is being expanded to work with the metadata around a Metric. It could be useful if Remap could work with the actual Metric value as well.

The complication here is that there are several different metric types. We could provide access according to the type. So:

.counter.value would return or set the value for counters.
.gauge.value would return or set the value for gauges.
.distribution.values would return or set an array for the distribution values.

We need to decide what to do if you, for example access .gauge.value when the metric is a counter? Is this an Error, or should it just return Null, or 0?

Should we only provide read only access to these values, or should we allow modification?

Should we allow the script to change the metric type? So the script could take a Gauge in could output a Distribution? If this is possible, we may then want to allow data to persist over calls. So 10 Gauge inputs could result in a single Distribution being output?

Will this ever be something we want to do?

@StephenWakely StephenWakely added type: enhancement A value-adding code change that enhances its existing functionality. transform: remap Anything `remap` transform related labels Dec 12, 2020
@binarylogic binarylogic added meta: idea Anything in the idea phase. Needs further discussion and consensus before work can begin. needs: approval Needs review & approval before work can begin. labels Dec 12, 2020
@binarylogic
Copy link
Contributor

Yeah this is a good question. I think we should wait and see if we get demand for this.

@binarylogic binarylogic added the needs: more demand Needs more demand before work can begin, +1 or comment to support. label Dec 12, 2020
jszwedko added a commit that referenced this issue Sep 22, 2021
Would address #5521 and #8105

This intially just adds support for accessing and modifying counter and
gauge values. Adding support for accessing and modifying distributions,
aggregated histograms, and aggregated summaries would be trickier, but
doable if we just allow accessing the components of each of these (for
example the raw array of samples for distributions, the buckets for
histograms). I wanted to get feedback on the general approach before
continuing though.

To answer some of the questions on the original issue:

> We need to decide what to do if you, for example access .gauge.value
when the metric is a counter? Is this an Error, or should it just return
Null, or 0?

I think we just return null. This is perhaps not idea, but consistent
with how we handle other undefined fields on metrics.

> Should we only provide read only access to these values, or should we
allow modification?

This is a good question. We could punt on allowing modification until
someone asks for it. I'm not aware of any current use cases for
modifying metric values.

> Should we allow the script to change the metric type? So the script
could take a Gauge in could output a Distribution? If this is possible,
we may then want to allow data to persist over calls. So 10 Gauge inputs
could result in a single Distribution being output?

I think this would be better handled by metric aggregation capabilities
via the `aggregate` and possibly future transforms.

> Will this ever be something we want to do?

I think yes. We've had a couple of people want to access the value for
unit tests.

Signed-off-by: Jesse Szwedko <jesse@szwedko.me>
@NasAmin
Copy link

NasAmin commented May 24, 2022

It would be great to get an update on this.
We are doing a remote_write from prometheus to vector and it seems to be converting counter to gauge in vector. Would be great if the metric type can be correctly detected out of the box or if we can change the type in vector 🙇

@FRosner
Copy link

FRosner commented May 24, 2022

@NasAmin you should be able to work around this with Lua as per #10171 (comment)

@NasAmin
Copy link

NasAmin commented May 24, 2022

Awesome thanks! Just wondering why the metrics type changes? I am assuming its the prometheus remote_write not specifying metrics type and vector defaulting to gauge?

@FRosner
Copy link

FRosner commented May 24, 2022

Yep, see #10171 (comment) and #10171 (comment).

@NasAmin
Copy link

NasAmin commented Oct 12, 2022

Just coming back to this, we have a sufficiently large instance of prometheus collecting 1000s of metrics. The metric names are not standardised.

It will be impossible to detect or guess the metric type arriving from prometheus_remote_write and set the correct type in Vector.

I am looking at prometheus_scrape as an alternative but I am not sure if that will scale because we rely heavily on the Prometheus operator CRDs for discovery of PrometheuRules and ServiceMonitors.

Have I hit a wall or is there a better way?

Thanks!

@NasAmin
Copy link

NasAmin commented Nov 9, 2022

👋 Will changing metric value and type be supported any time soon in VRL to avoid using Lua?

@jszwedko
Copy link
Member

jszwedko commented Nov 9, 2022

👋 Will changing metric value and type be supported any time soon in VRL to avoid using Lua?

No immediate plans unfortunately. It is in our backlog though.

@bbeaudreault
Copy link

Any update here? I think the essential piece is providing some way at all to unit test values being output by vector. It'd be nice if that were handled in a unified way with remap, but probably not a strict requirement?

@jszwedko
Copy link
Member

Any update here? I think the essential piece is providing some way at all to unit test values being output by vector. It'd be nice if that were handled in a unified way with remap, but probably not a strict requirement?

Unfortunately no updates yet. This is a known gap 😦

@curreli
Copy link

curreli commented Dec 6, 2023

Hello,

I also need to be able to modify the metric event, as in its current form, it is really not practical to use within OpenSearch Dashboards as every field is named gauge.value or counter.value and it's not possible to differentiate between two different fields.

For example, I want to be able to compute the normalized 1min load as follows:

Screenshot 2023-12-06 at 22 37 16 Screenshot 2023-12-06 at 22 37 33

But load1 and physical_cpus both hold their value under the field name gauge.value and I have no way to differentiate between the two.

Screenshot 2023-12-06 at 22 37 53

I'm not an advanced OpenSearch Dashboards user but I would love to have fields like host.load.load1.gauge.value and host.cpu.physical_cpus.gauge.value to be able to easily select them and mix them together in computations.

I have tried adding a field with a lua transform but it seems we cannot even add a key at the root of the event:

transformed_metrics:
  type: lua
  version: "2"
  inputs:
    - "*_metrics"
  hooks:
    process: |-
      function (event, emit)
        event.metric[event.metric.name] = event.metric.gauge.value
        emit(event)
      end

This does not produce any change in the event. The only way I found to add a field was adding it as a tag but there I had to transform the type to a string which does not make sense (and it does not make sense to put a value field in a tag anyway).

Could you please advise on a solution to my problem? Have I missed something? If we cannot modify the metric event, could we at least fully customize how we want to send data through the Elasticsearch sink?

Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: idea Anything in the idea phase. Needs further discussion and consensus before work can begin. needs: approval Needs review & approval before work can begin. transform: remap Anything `remap` transform related type: enhancement A value-adding code change that enhances its existing functionality.
Projects
None yet
Development

No branches or pull requests

7 participants