-
Notifications
You must be signed in to change notification settings - Fork 208
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
influxdb2otel receiver #1760
base: main
Are you sure you want to change the base?
influxdb2otel receiver #1760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs look good. Only a couple really minor suggestions.
docs/sources/reference/components/otelcol/otelcol.receiver.influxdb.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.influxdb.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.influxdb.md
Outdated
Show resolved
Hide resolved
…luxdb.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…luxdb.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
…luxdb.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your contribution :) I left a few comments.
The config converter is also missing, have a look at this PR as example:
func init() { | ||
component.Register(component.Registration{ | ||
Name: "otelcol.receiver.influxdb", | ||
Stability: featuregate.StabilityExperimental, // Adjust the stability level if needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upstream stability is "beta" which is pretty stable in Otel standards. I think that it's ok to set it to GenerallyAvailable
endpoint = "localhost:8086" // InfluxDB metrics ingestion endpoint | ||
|
||
output { | ||
metrics = [otelcol.exporter.prometheus.influx_output.input] // Forward metrics to Prometheus exporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
converting Otel metrics to Prometheus metrics is possible but often unnecessary if the endpoint supports OTLP. Mimir supports OTLP and we also have an OTLP gateway that supports all signals. Here I would just put "metrics = [...]" and in the examples below it's fine to have an example that converts to prometheus but I would put first an example that uses the otlphttp exporter
|
||
| Name | Type | Description | Default | Required | | ||
| -------------------- | -------------- | ---------------------------------------------------------------- | ------------------ | -------- | | ||
| `endpoint` | `string` | `host:port` to listen for traffic on. | `"localhost:8086"` | no | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation is missing for the full http server config (args and blocks). See the zipkin receiver config as example
@@ -94,6 +94,7 @@ import ( | |||
_ "github.com/grafana/alloy/internal/component/otelcol/processor/tail_sampling" // Import otelcol.processor.tail_sampling | |||
_ "github.com/grafana/alloy/internal/component/otelcol/processor/transform" // Import otelcol.processor.transform | |||
_ "github.com/grafana/alloy/internal/component/otelcol/receiver/datadog" // Import otelcol.receiver.datadog | |||
_ "github.com/grafana/alloy/internal/component/otelcol/receiver/influxdb" // Import otelcol.exporter.datadog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment is wrong here
title: otelcol.receiver.influxdb | ||
--- | ||
|
||
<span class="badge docs-labels__stage docs-labels__item">Experimental</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we put it generally available we should remove the span here
@@ -813,9 +813,21 @@ require ( | |||
) | |||
|
|||
require ( | |||
github.com/influxdata/influxdb-client-go/v2 v2.14.0 | |||
github.com/influxdata/influxdb1-client v0.0.0-20220302092344-a9ab5670611c | |||
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/influxdbreceiver v0.108.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be better to put the requirement next to the other otel components requirements to keep them grouped
@@ -52,7 +52,7 @@ v1.4.0 | |||
- (_Experimental_) Add an `otelcol.processor.interval` component to aggregate metrics and periodically | |||
forward the latest values to the next component in the pipeline. | |||
- (_Experimental_) Add a `loki.secretfilter` component to redact secrets from collected logs. | |||
|
|||
- Added InfluxDB receiver that converts influx metric into OTEL. (@EHSchmitt4395) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is misplaced, it should be in the "Main" area above
PR Description
Influx receiver that converts to OTEL
Can be used in conjunction with
otelcol.exporter.prometheus
to further convert influx to prom format.ex alloy config:
Notes to the Reviewer
plz let me know if issue or something missing with my pr - cheers :)
PR Checklist