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.

Sign up for GitHub

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

Jump to bottom

prometheusremotewrite exporter crashes collector with metrics names < 6 characters #2608

Closed
dan-vectra opened this issue Mar 5, 2021 · 0 comments · Fixed by #2613
Closed

prometheusremotewrite exporter crashes collector with metrics names < 6 characters #2608

dan-vectra opened this issue Mar 5, 2021 · 0 comments · Fixed by #2613
Labels
bug Something isn't working

Comments

@dan-vectra
Copy link

Describe the bug
When Collector uses prometheusremotewriteexporter, metric names longer than 6 characters cause it to crash

2021-03-03 09:30:54
panic: runtime error: slice bounds out of range [-1:]

2021-03-03 09:30:54
goroutine 97 [running]:

2021-03-03 09:30:54
go.opentelemetry.io/collector/exporter/prometheusremotewriteexporter.getPromMetricName(0xc0003df7c0, 0x0, 0x0, 0x6, 0x6)

Seems to be caused by this logic which checks for "total" at the end of the metric name:

opentelemetry-collector/exporter/prometheusremotewriteexporter/helper.go

Line 197 in 68c5961

isCounter = isCounter && name[len(name)-len(totalStr):] != totalStr

Steps to reproduce
Enable Prometheus Remote Write exporter, send OTLP metrics to collector with names shorter than 6 characters

What version did you use?
Version: 0.16.0

What config did you use?
Config:
extensions:
health_check:
port: 13133

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

processors:
batch:

exporters:
logging:
logLevel: warn
prometheusremotewrite:
endpoint: "http://127.0.0.1:9201/write"

service:
pipelines:
traces:
receivers: [otlp]
processors: [batch]
exporters: [logging]

metrics:
  receivers: [otlp]
  processors: [batch]
  exporters: [logging, prometheusremotewrite]

extensions: [health_check]

Environment
OS: alpine:3.12 (Docker)
@dan-vectra dan-vectra added the bug Something isn't working label Mar 5, 2021
naseemkullah pushed a commit to naseemkullah/opentelemetry-collector that referenced this issue Mar 6, 2021
[prometheusremotewrite] fix: counter name check
a38add0 
fixes  open-telemetry#2608

Ensure that the metric name is greater than least the length of the
suffix ("_total") before checking if it contains the suffix to prevent
crashes for short (smaller or eq to the length of the suffix) metric names.

Signed-off-by: naseemkullah <naseem@transit.app>
@naseemkullah naseemkullah mentioned this issue Mar 6, 2021
Merged
naseemkullah pushed a commit to naseemkullah/opentelemetry-collector that referenced this issue Mar 6, 2021
[prometheusremotewrite] fix: counter name check
8bffb8e 
fixes  open-telemetry#2608

Ensure that the metric name is greater than the length of the
counter suffix ("_total") before checking if it contains the counter
suffix to prevent crashes for short (smaller or eq to the length of the
suffix) metric names.

The edge case where the metric name is "_total" is not handled (considered
not to have the suffix), but should not occur IRL.

Also fix the spelling of "delimiter".

Signed-off-by: naseemkullah <naseem@transit.app>
bogdandrutu pushed a commit that referenced this issue Mar 8, 2021
[prometheusremotewrite] fix: counter name check (#2613)
35cf41e 
* [prometheusremotewrite] fix: counter name check

fixes  #2608

Ensure that the metric name is greater than the length of the
counter suffix ("_total") before checking if it contains the counter
suffix to prevent crashes for short (smaller or eq to the length of the
suffix) metric names.

The edge case where the metric name is "_total" is not handled (considered
not to have the suffix), but should not occur IRL.

Also fix the spelling of "delimiter".

Signed-off-by: naseemkullah <naseem@transit.app>

* use strings.HasSuffix()

Signed-off-by: naseemkullah <naseem@transit.app>

* add already suffixed counter test

Signed-off-by: naseemkullah <naseem@transit.app>
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this issue Apr 27, 2023
@atoulme
Add an attribute value check (open-telemetry#2608)
50c4235
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this issue Jul 5, 2024
@keith
[BUILD] Add support for bzlmod (open-telemetry#2608)
025f42f 
* Add support for bzlmod

This adds support for bzlmod, which is bazel's new dependency resolution
system.

Theoretically we could now remove the previous dependency management
configuration which required flattening all the dependencies, but I left
it here for now for users using old versions of bazel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[prometheusremotewrite] fix: counter name check naseemkullah/opentelemetry-collector
1 participant
@dan-vectra