lib/protoparser: adds opentelemetry parser #2570

f41gh7 · 2022-05-11T19:23:09Z

app/{vmagent,vminsert}: adds opentelemetry ingestion path

Adds ability to ingest data with opentelemetry protocol
protobuf and json encoding is supported
data converted into prometheus protobuf timeseries
each data type has own converter and it may produce multiple timeseries
from single datapoint (for summary and histogram).
only cumulative aggregationFamily is supported for sum(prometheus
counter) and histogram.

For #2424

hagen1778

LGTM

README.md

app/vminsert/opentelemetryhttp/request_handler.go

codecov · 2022-06-02T16:39:53Z

Codecov Report

Patch coverage: 76.09% and project coverage change: -3.48% ⚠️

Comparison is base (7e5555f) 61.06% compared to head (532ec90) 57.58%.
Report is 1 commits behind head on master.

❗ Current head 532ec90 differs from pull request most recent head 4ae796f. Consider uploading reports for the commit 4ae796f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2570      +/-   ##
==========================================
- Coverage   61.06%   57.58%   -3.48%     
==========================================
  Files         385      310      -75     
  Lines       71951    52184   -19767     
==========================================
- Hits        43939    30052   -13887     
+ Misses      25605    20237    -5368     
+ Partials     2407     1895     -512

Files Changed	Coverage Δ
lib/protoparser/opentelemetry/streamparser.go	`76.09% <76.09%> (ø)`

... and 280 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hagen1778

LGTM

valyala · 2022-08-22T07:50:26Z

This pull request increases VictoriaMetrics binary size from 19MB to 29MB, e.g. by around 50%. This is too big price to pay for the inclusion of OpenTelemetry-compatible JSON and protobuf data ingestion protocols :( All the existing data ingestion protocols supported by VictoriaMetrics increase binary size by a few tens of kilobytes in worst case. It would be great trying to decrease the binary size overhead for this change before merging it into VictoriaMetrics.

In the mean time it is possible to export data from OpenTelemetry collector client to VictoriaMetrics via one of the supported data ingestion protocols:

valyala

See #2570 (comment)

rodrigc · 2022-09-15T16:36:19Z

@valyala might be nice to add to the VictoriaMetrics docs, some
example configs (or pointers to configs in OpenTelemetry docs) for configuring an OpenTelemetry client to push to VictoriaMetrics via the various protocols you mentioned. That way end-users would have a better idea how to configure OpenTelemetry to push to VictoriaMetrics. If the existing protocols work well enough, then a native OLTP ingest path may not be necessary.

If native OLTP could be implemented without increasing the footprint requirements for VictoriaMetrics, that would be nice too, but is a longer discussion/implementation.

syepes · 2022-09-16T12:25:41Z

+1 Hope you can find a solution to this one, this would be a great addition.

syepes · 2022-11-08T07:59:23Z

@f41gh7 Is there any news on the merger of this feature? it would be great to have native support for this new protocol

PatMis16 · 2022-11-22T06:46:49Z

Hi VictoriaMetrics Developers!
How about this feture with the OpenTelemetry protocol? Is there a planned release date for this?

f41gh7 · 2022-11-22T17:07:03Z

Will take a look and update dependency soon.

f41gh7 · 2022-11-29T18:31:38Z

@valyala PR was updated, but any way it increases result binary size by 5MB because of protobuf json parser dependencies. It's possible to optimize it further, but I'm not sure, that it worth it.

valyala · 2022-12-14T19:53:18Z

PR was updated, but any way it increases result binary size by 5MB because of protobuf json parser dependencies. It's possible to optimize it further, but I'm not sure, that it worth it.

+5MB is better than +10MB from the previous attempt, but is is still too much for a simple parser for metrics in OpenTelemetry protocol when comparing to binary footprints for other data ingestion protocols supported by VictoriaMetrics :(

I believe we can write our own implementation for the OpenTelemetry metrics parser, which will increase the resulting binary size by no more than 100Kb in the worst case.

f41gh7 · 2022-12-28T20:16:20Z

@valyala changed implementation - rewritten parser from scratch with default protobuf generator. Produced binary size almost the same as it was before adding this feature.

syepes · 2023-01-06T22:07:38Z

@f41gh7 Do you guys have an rough idea when you will be releasing a version with this?

lib/protoparser/opentelemetry/pb/json_parser.go

valyala · 2023-01-10T23:24:11Z

Do you guys have an rough idea when you will be releasing a version with this?

We had plans to merge this pull request in the upcoming v1.86.0 release, but it won't be included there because of issues with the increased binary size :( The latest version of the pull request still increases VictoriaMetrics binary size by 4MiB from 19MiB to 23MiB. I hope these issues will be resolved until the next release, so the support for OpenTelemetry protocol will be added into the next release after v1.86.0.

Side note: the OpenTelemetry metrics protocol specification is an over-engineered nightmare comparing to other popular data ingestion formats such as Influx line protocol, Graphite plaintext protocol and Prometheus text exposition format. This introduces non-trivial overhead to users, clients and server, which have to deal with it. For example, a simple temperature{job="vm",label1="value1"} 15 metric transforms into the following monster when being transferred with OpenTelemetry metrics JSON format:

{
  "resourceMetrics": [
    {
      "resource": {
        "attributes": [
          {
            "key": "job",
            "value": {
              "stringValue": "vm"
            }
          }
        ]
      },
      "scopeMetrics": [
        {
          "metrics": [
            {
              "name": "temperature",
              "gauge": {
                "dataPoints": [
                  {
                    "attributes": [
                      {
                        "key": "label1",
                        "value": {
                          "stringValue": "value1"
                        }
                      }
                    ],
                    "asInt": "15"
                  }
                ]
              }
            }
          ]
        }
      ]
    }
  ]
}

f41gh7 · 2023-02-27T17:35:53Z

@valyala changed proto compiler. Without any external dependency it increases binary size for 1MB approx.

hagen1778 · 2023-02-27T20:15:51Z

Without any external dependency it increases binary size for 1MB approx.

not all heroes wear capes)

rodrigc · 2023-03-11T00:26:12Z

@hagen1778 @f41gh7 Small PR to fix the lint issues in this branch: #3943

PatMis16 · 2023-03-25T09:38:26Z

Great work guys! Hope this is comming soon.

syepes · 2023-04-14T14:51:37Z

Any good news on this merger?

zahdab · 2023-05-16T13:07:34Z

Any update on this by any chance?

app/{vmagent,vminsert}: adds opentelemetry ingestion path Adds ability to ingest data with opentelemetry protocol protobuf and json encoding is supported data converted into prometheus protobuf timeseries each data type has own converter and it may produce multiple timeseries from single datapoint (for summary and histogram). only cumulative aggregationFamily is supported for sum(prometheus counter) and histogram. Apply suggestions from code review Co-authored-by: Roman Khavronenko <roman@victoriametrics.com> updates deps fixes tests wip wip wip wip lib/protoparser/opentelemetry: moves to vtprotobuf generator go mod vendor lib/protoparse/opentelemetry: reduce memory allocations

syepes · 2023-06-26T17:50:29Z

Closer and closer to have this merged, there will be a big party for this one 🥳

Cho1409 · 2023-07-17T19:46:44Z

I hope to see some updates soon.

- Remove support for JSON parsing, since it is too fragile and is rarely used in practice. The most clients send OpenTelemetry metrics in protobuf. The JSON parser can be added in the future if needed. - Remove unused code from lib/protoparser/opentelemetry/pb and lib/protoparser/opentelemetry/proto - Do not re-use protobuf message between ParseStream() calls, since there is high chance of high fragmentation of the re-used message because of too complex nested structure of the message.

valyala · 2023-07-27T20:27:00Z

@f41gh7 , thanks for the pull request!

* lib/protoparser: adds opentelemetry parser app/{vmagent,vminsert}: adds opentelemetry ingestion path Adds ability to ingest data with opentelemetry protocol protobuf and json encoding is supported data converted into prometheus protobuf timeseries each data type has own converter and it may produce multiple timeseries from single datapoint (for summary and histogram). only cumulative aggregationFamily is supported for sum(prometheus counter) and histogram. Apply suggestions from code review Co-authored-by: Roman Khavronenko <roman@victoriametrics.com> updates deps fixes tests wip wip wip wip lib/protoparser/opentelemetry: moves to vtprotobuf generator go mod vendor lib/protoparse/opentelemetry: reduce memory allocations * wip - Remove support for JSON parsing, since it is too fragile and is rarely used in practice. The most clients send OpenTelemetry metrics in protobuf. The JSON parser can be added in the future if needed. - Remove unused code from lib/protoparser/opentelemetry/pb and lib/protoparser/opentelemetry/proto - Do not re-use protobuf message between ParseStream() calls, since there is high chance of high fragmentation of the re-used message because of too complex nested structure of the message. * wip * wip * wip --------- Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>

hagen1778 · 2023-07-27T22:17:12Z

No way!

valyala · 2023-07-28T00:46:19Z

FYI, this pull request has been included in VictoriaMetrics v1.92.0.

sumukhs · 2023-08-01T23:12:43Z

Hi @valyala ,

When I use opentelemetry-sdk python version 1.19.0 and generate a few metrics in json and try to push this to Victoria Metrics instance version v1.92.0 I see an error like this:

2023-08-01T22:43:01.654Z        warn    VictoriaMetrics/app/vminsert/main.go:218        remoteAddr: "172.17.0.1:45372"; requestURI: /opentelemetry/api/v1/push; cannot unpack OpenTelemetry metrics: cannot unmarshal request from 4738 bytes: proto: illegal wireType 6

The way I get the json data for the metrics is by dumping the state in a file like this:

    reader = InMemoryMetricReader()
    provider = MeterProvider(metric_readers=[reader])
    set_meter_provider(provider)

    meter = get_meter_provider().get_meter("MIT-Metrics", "0.1.2")
    state = reader.get_metrics_data().to_json()
    logging.info(f"State:\n {state}")
    with open(output_file, "w") as f:
        json.dump(state, f)

Is this a supported way of reporting metrics into Victoria metrics?

f41gh7 · 2023-08-03T09:42:25Z

@sumukhs VictoriaMetrics supports opentelemetry only with protobuf encoding. json isn't supported atm.

valyala · 2023-08-12T14:51:52Z

FYI, VictoriaMetrics v1.93.0 has the following enhancements for Opentelemetry:

It returns user-readable error on an attempt to push OTEL metrics with unsupported encoding. See this pull request.
It prevents from panic when pushing histograms with non-counter buckets. See this issue for details.

…to for protobuf message unmarshaling and marshaling This reduces VictoriaMetrics binary size by 100KB. Updates #2570 Updates #2424

valyala · 2024-01-14T19:52:23Z

FYI, the commit dd25049 switches parsing of OTEL protobuf messages from protoc-generated code to github.com/VictoriaMetrics/easyproto. This reduces code bloat and binary bloat (VictoriaMetrics binary size has been reduced by 100KB on top of this change), and improves maintainability of the resulting code. This commit will be included in the next release of VictoriaMetrics.

…to for protobuf message unmarshaling and marshaling This reduces VictoriaMetrics binary size by 100KB. Updates #2570 Updates #2424

f41gh7 marked this pull request as ready for review May 11, 2022 19:23

f41gh7 requested review from valyala and hagen1778 May 11, 2022 19:23

hagen1778 reviewed Jun 2, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

app/vminsert/opentelemetryhttp/request_handler.go Outdated Show resolved Hide resolved

f41gh7 force-pushed the adds-opentelemetry branch 2 times, most recently from 0439bdf to 88c4b16 Compare June 2, 2022 16:30

f41gh7 mentioned this pull request Jun 2, 2022

OTLP support in VictoriaMetrics? #2424

Closed

hagen1778 approved these changes Jun 2, 2022

View reviewed changes

valyala requested changes Aug 22, 2022

View reviewed changes

f41gh7 force-pushed the adds-opentelemetry branch from a1990f0 to c6f3384 Compare November 29, 2022 16:34

f41gh7 requested a review from valyala November 29, 2022 18:31

f41gh7 force-pushed the adds-opentelemetry branch from c6f3384 to c17d5b1 Compare December 28, 2022 20:13

f41gh7 force-pushed the adds-opentelemetry branch from c17d5b1 to db514d9 Compare December 28, 2022 20:20

github-advanced-security bot found potential problems Jan 10, 2023

View reviewed changes

lib/protoparser/opentelemetry/pb/json_parser.go Fixed Show fixed Hide fixed

lib/protoparser/opentelemetry/pb/json_parser.go Fixed Show fixed Hide fixed

lib/protoparser/opentelemetry/pb/json_parser.go Fixed Show fixed Hide fixed

f41gh7 force-pushed the adds-opentelemetry branch from 4fa7282 to 7dfec41 Compare February 27, 2023 17:29

f41gh7 force-pushed the adds-opentelemetry branch 2 times, most recently from 85a3381 to 8d10087 Compare June 26, 2023 08:24

f41gh7 force-pushed the adds-opentelemetry branch from 8d10087 to b3c955a Compare June 26, 2023 08:25

valyala mentioned this pull request Jul 21, 2023

app/vlinsert: add support of loki push protocol #4482

Merged

valyala added 4 commits July 26, 2023 15:29

Merge branch 'public-single-node' into adds-opentelemetry

7ea780c

Merge branch 'public-single-node' into adds-opentelemetry

fce3c8c

wip

083b99a

valyala approved these changes Jul 27, 2023

View reviewed changes

valyala added 2 commits July 27, 2023 13:22

wip

5b90a25

wip

4ae796f

valyala merged commit 46ecbbe into master Jul 27, 2023
6 checks passed

valyala deleted the adds-opentelemetry branch July 27, 2023 20:26

JNKielmann mentioned this pull request Aug 10, 2023

Runtime panic during OpenTelemetry ingestion #4814

Closed

valyala added a commit that referenced this pull request Jan 14, 2024

lib/protoparser/opentelemetry: use github.com/VictoriaMetrics/easypro…

dd25049

…to for protobuf message unmarshaling and marshaling This reduces VictoriaMetrics binary size by 100KB. Updates #2570 Updates #2424

valyala added a commit that referenced this pull request Jan 16, 2024

lib/protoparser/opentelemetry: use github.com/VictoriaMetrics/easypro…

f8ae2ab

…to for protobuf message unmarshaling and marshaling This reduces VictoriaMetrics binary size by 100KB. Updates #2570 Updates #2424

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/protoparser: adds opentelemetry parser #2570

lib/protoparser: adds opentelemetry parser #2570

f41gh7 commented May 11, 2022 •

edited by valyala

hagen1778 left a comment

codecov bot commented Jun 2, 2022 •

edited

hagen1778 left a comment

valyala commented Aug 22, 2022

valyala left a comment

rodrigc commented Sep 15, 2022

syepes commented Sep 16, 2022

syepes commented Nov 8, 2022

PatMis16 commented Nov 22, 2022

f41gh7 commented Nov 22, 2022

f41gh7 commented Nov 29, 2022

valyala commented Dec 14, 2022

f41gh7 commented Dec 28, 2022

syepes commented Jan 6, 2023

valyala commented Jan 10, 2023 •

edited

f41gh7 commented Feb 27, 2023

hagen1778 commented Feb 27, 2023

rodrigc commented Mar 11, 2023

PatMis16 commented Mar 25, 2023

syepes commented Apr 14, 2023

zahdab commented May 16, 2023

syepes commented Jun 26, 2023

Cho1409 commented Jul 17, 2023

valyala commented Jul 27, 2023

hagen1778 commented Jul 27, 2023

valyala commented Jul 28, 2023

sumukhs commented Aug 1, 2023 •

edited

f41gh7 commented Aug 3, 2023

valyala commented Aug 12, 2023

valyala commented Jan 14, 2024

lib/protoparser: adds opentelemetry parser #2570

lib/protoparser: adds opentelemetry parser #2570

Conversation

f41gh7 commented May 11, 2022 • edited by valyala

hagen1778 left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 2, 2022 • edited

Codecov Report

hagen1778 left a comment

Choose a reason for hiding this comment

valyala commented Aug 22, 2022

valyala left a comment

Choose a reason for hiding this comment

rodrigc commented Sep 15, 2022

syepes commented Sep 16, 2022

syepes commented Nov 8, 2022

PatMis16 commented Nov 22, 2022

f41gh7 commented Nov 22, 2022

f41gh7 commented Nov 29, 2022

valyala commented Dec 14, 2022

f41gh7 commented Dec 28, 2022

syepes commented Jan 6, 2023

valyala commented Jan 10, 2023 • edited

f41gh7 commented Feb 27, 2023

hagen1778 commented Feb 27, 2023

rodrigc commented Mar 11, 2023

PatMis16 commented Mar 25, 2023

syepes commented Apr 14, 2023

zahdab commented May 16, 2023

syepes commented Jun 26, 2023

Cho1409 commented Jul 17, 2023

valyala commented Jul 27, 2023

hagen1778 commented Jul 27, 2023

valyala commented Jul 28, 2023

sumukhs commented Aug 1, 2023 • edited

f41gh7 commented Aug 3, 2023

valyala commented Aug 12, 2023

valyala commented Jan 14, 2024

f41gh7 commented May 11, 2022 •

edited by valyala

codecov bot commented Jun 2, 2022 •

edited

valyala commented Jan 10, 2023 •

edited

sumukhs commented Aug 1, 2023 •

edited