close
Skip to content

fix(otlp-transformer): do not throw when deserializing empty JSON response - #5551

Merged
pichlermarc merged 4 commits into
open-telemetry:mainfrom
dynatrace-oss-contrib:fix/empty-json-response-throws
Apr 14, 2025
Merged

fix(otlp-transformer): do not throw when deserializing empty JSON response#5551
pichlermarc merged 4 commits into
open-telemetry:mainfrom
dynatrace-oss-contrib:fix/empty-json-response-throws

Conversation

@pichlermarc

Copy link
Copy Markdown
Member

Which problem is this PR solving?

When receiving an empty response form the server, the de-serialization code would throw, which caused a log message to be written. This PR changes it so that empty responses from the server are treated as such.

Fixes #5548

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Added Unit tests

@pichlermarc pichlermarc added bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization labels Mar 20, 2025
@codecov

codecov Bot commented Mar 20, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.95%. Comparing base (4f9b31b) to head (a5f97fc).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5551   +/-   ##
=======================================
  Coverage   94.95%   94.95%           
=======================================
  Files         309      309           
  Lines        7946     7952    +6     
  Branches     1608     1611    +3     
=======================================
+ Hits         7545     7551    +6     
  Misses        401      401           
Files with missing lines Coverage Δ
...al/packages/otlp-transformer/src/logs/json/logs.ts 100.00% <100.00%> (ø)
...kages/otlp-transformer/src/metrics/json/metrics.ts 100.00% <100.00%> (ø)
.../packages/otlp-transformer/src/trace/json/trace.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pichlermarc

Copy link
Copy Markdown
Member Author

Actually I'm not sure if an empty response is a valid OTLP response or not. It might now be a spec compliant response after all since technically an empty string is not valid JSON.

Related spec: https://opentelemetry.io/docs/specs/otlp/#full-success-1

@pichlermarc
pichlermarc marked this pull request as ready for review March 20, 2025 13:35
@pichlermarc
pichlermarc requested a review from a team as a code owner March 20, 2025 13:35
@pichlermarc
pichlermarc added this pull request to the merge queue Apr 14, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 14, 2025
@pichlermarc
pichlermarc added this pull request to the merge queue Apr 14, 2025
Merged via the queue into open-telemetry:main with commit 37fe1e4 Apr 14, 2025
@pichlermarc
pichlermarc deleted the fix/empty-json-response-throws branch April 14, 2025 08:21
pichlermarc added a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pkg:otlp-transformer priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTLPExportDelegate Export succeeded but could not deserialize response - is the response specification compliant

3 participants