search

influxdata / telegraf

Agent for collecting, processing, aggregating, and writing metrics, logs, and other arbitrary data.
https://influxdata.com/telegraf
MIT License
14.68k stars 5.59k forks source link

Unexpected metric name for empty `name` setting in GNMI #13380

Closed protonmarco closed 1 year ago

protonmarco commented 1 year ago

This is a follow-up of #12352 where the metric name produced by 

  [[inputs.gnmi.subscription]]
    name = ""
    origin = "openconfig-interfaces"
    path = "/interfaces"
    subscription_mode = "sample"
    sample_interval = "10s"

or

[[inputs.gnmi.subscription]]
  name = ""
  origin = "openconfig-interfaces"
  path = "/interfaces/interface/state/counters"
  subscription_mode = "sample"
  sample_interval = "10s"

is set to the last path element. This is unexpected as we should either error out on an empty name or use a default name such as gnmi.

srebhan commented 1 year ago

@protonmarco seems like I misunderstood your issue... Sorry for that! To your problem: And empty metric name is not allowed in Telegraf! While the actual behavior is unexpected and should be fixed (e.g. by checking that name is not-empty or by using a default name like gnmi) we always need a metric name!

However, we might add an option to outputs.prometheus_client to not prefix the field with the metric name!

protonmarco commented 1 year ago

no problem :)

we always need a metric name!

Then I think the best solution would be for us to keep the name field not empty and fill it with "gnmi" or something else of our choice. I fear that removing the prefix indiscriminately on the outputs side opens to edge cases where you have the prefix removed from everywhere but maybe it is only needed for certain devices etc.. I would close this issue as I don't think we want to pursue this path and using a custom prefix is an easy fix.

srebhan commented 1 year ago

I think we should keep this open as currently the produced GNMI name is unexpected at best. :-) I will change the issue a bit but there is an issue so let's track it.

Regarding the output, we could have a flag, say omit_metric_name_prefix that you can set... Not sure if you want this or live with the name prefix...

protonmarco commented 1 year ago

alright. I think it's fine to live with the name prefix set, putting a flag on the outputs side to be used just for gnmi inputs without the ability to apply it only to a selected set of .conf doesn't sound good imo, especially because we can bypass this problem setting a name. I'm thinking at my implementation where I have a .conf file for each device

srebhan commented 1 year ago

Ok. Will try to get this done...