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

[connector/otlpjson] Invalid OTLP payload is silently ignored #35739

Closed
andrzej-stencel opened this issue Oct 11, 2024 · 3 comments · Fixed by #35827
Closed

[connector/otlpjson] Invalid OTLP payload is silently ignored #35739

andrzej-stencel opened this issue Oct 11, 2024 · 3 comments · Fixed by #35827
Labels
bug Something isn't working connector/otlpjson Stale

Comments

@andrzej-stencel
Copy link
Member

Component(s)

connector/otlpjson

What happened?

Description

The OTLP/JSON connector reports errors on invalid JSON, but silently ignores valid JSON that is not valid OTLP. I would expect it to also error out on valid JSON that is not valid OTLP. As another enhancement, an error_mode option could be added for users to configure the behavior.

Steps to Reproduce

  1. Prepare a newline-delimited JSON file like the below non-otlp.json file:

    {"one":"two"}
    {"this":{"is": {"not": "otlp"}}}
  2. Run the collector with the below configuration, reading the file with Filelog receiver and passing it to OTLP/JSON connector and to Debug exporter.

Expected Result

The OTLP/JSON connector rejects the input lines, outputting an error message saying that the input is not valid OTLP.

Actual Result

The connector does not report any errors. It emits an empty batch for each invalid OTLP line (see related issue #35738).

Collector version

v0.111.0

Environment information

Environment

OS: Ubuntu 24.04

OpenTelemetry Collector configuration

connectors:
  otlpjson:
exporters:
  debug:
    verbosity: basic
receivers:
  filelog:
    include:
      - non-otlp.json
    start_at: beginning
service:
  pipelines:
    logs/input:
      exporters: [otlpjson]
      receivers: [filelog]
    logs/otlp:
      exporters: [debug]
      receivers: [otlpjson]

Log output

2024-10-11T12:40:27.217+0200    info    service@v0.111.0/service.go:136 Setting up own telemetry...
2024-10-11T12:40:27.217+0200    info    telemetry/metrics.go:70 Serving metrics {"address": "localhost:8888", "metrics level": "Normal"}
2024-10-11T12:40:27.217+0200    info    builders/builders.go:26 Development component. May change in the future.        {"kind": "exporter", "data_type": "logs", "name": "debug"}
2024-10-11T12:40:27.218+0200    info    otlpjsonconnector@v0.111.0/logs.go:27   Building otlpjson connector for logs    {"kind": "connector", "name": "otlpjson", "exporter_in_pipeline": "logs", "receiver_in_pipeline": "logs"}
2024-10-11T12:40:27.219+0200    info    service@v0.111.0/service.go:208 Starting otelcol-contrib...     {"Version": "0.111.0", "NumCPU": 20}
2024-10-11T12:40:27.219+0200    info    extensions/extensions.go:39     Starting extensions...
2024-10-11T12:40:27.219+0200    info    adapter/receiver.go:47  Starting stanza receiver        {"kind": "receiver", "name": "filelog", "data_type": "logs"}
2024-10-11T12:40:27.219+0200    info    service@v0.111.0/service.go:234 Everything is ready. Begin running and processing data.
2024-10-11T12:40:27.420+0200    info    fileconsumer/file.go:256        Started watching file   {"kind": "receiver", "name": "filelog", "data_type": "logs", "component": "fileconsumer", "path": "non-otlp.json"}
2024-10-11T12:40:27.521+0200    info    LogsExporter    {"kind": "exporter", "data_type": "logs", "name": "debug", "resource logs": 0, "log records": 0}
2024-10-11T12:40:27.521+0200    info    LogsExporter    {"kind": "exporter", "data_type": "logs", "name": "debug", "resource logs": 0, "log records": 0}

Additional context

The Unmarshal(Logs|Metrics|Traces) methods do not return an error for valid JSON that is not a valid OTLP payload. They just return an empty object of logs/metrics/traces data. See https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.111.0/connector/otlpjsonconnector/traces.go#L55 for example.

@andrzej-stencel andrzej-stencel added bug Something isn't working needs triage New item requiring triage labels Oct 11, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@ChrsMark
Copy link
Member

ChrsMark commented Oct 11, 2024

Doesn't this fall into the same category with what the #35738 describes?

A ptrace given as an input to metrics' unmarshaller is an "invalid" otlp similar to what a very random JSON would be 🤔 .

@atoulme atoulme removed the needs triage New item requiring triage label Oct 12, 2024
andrzej-stencel pushed a commit to andrzej-stencel/opentelemetry-collector-contrib that referenced this issue Nov 8, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The connector now does not emit empty batches for invalid otlp payload
and throws an error instead. Approach discussed here
open-telemetry#35738 (comment)

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#35738 and open-telemetry#35739

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Manual Testing 

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
pull bot pushed a commit to abaguas/opentelemetry-collector-contrib that referenced this issue Nov 8, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The connector now does not emit empty batches for invalid otlp payload
and throws an error instead. Approach discussed here
open-telemetry#35738 (comment)

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#35738 and open-telemetry#35739

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Manual Testing 

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Dec 16, 2024
@ChrsMark ChrsMark linked a pull request Dec 16, 2024 that will close this issue
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this issue Dec 17, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
The connector now does not emit empty batches for invalid otlp payload
and throws an error instead. Approach discussed here
open-telemetry#35738 (comment)

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#35738 and open-telemetry#35739

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Manual Testing 

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working connector/otlpjson Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants