fix(telemetry): Remove DSN object from envelope header#2851
Open
we5 wants to merge 2 commits intogetsentry:masterfrom
Open
fix(telemetry): Remove DSN object from envelope header#2851we5 wants to merge 2 commits intogetsentry:masterfrom
we5 wants to merge 2 commits intogetsentry:masterfrom
Conversation
The DSN object was incorrectly included in the envelope headers for telemetry events (logs, metrics), causing serialization failures when the envelope was converted to JSON. The error occurred because JSON.generate() couldn't serialize the DSN object (a complex Ruby object) directly. The server rejected these envelopes with: invalid event envelope invalid type: map, expected a string at line 1 column 360 This fix removes the dsn parameter from the Envelope initialization in TelemetryEventBuffer#send_items, aligning it with how other envelope types (events, sessions) are created in the codebase, which don't include the DSN in their headers. The DSN is not needed in the envelope header as the transport layer already has access to it for routing the request to the correct Sentry endpoint. Fixes log ingestion when config.enable_logs is enabled.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
The previous fix incorrectly removed the DSN from the envelope header entirely. However, the DSN should be included but must be converted to a string using .to_s before being added to the header. This aligns with the existing implementation in Transport#envelope_from_event (transport.rb:122) which uses `dsn: @dsn.to_s`. Changes: - Add dsn back to envelope header in TelemetryEventBuffer#send_items, converting it to string with .to_s - Fix test in metrics_spec.rb to expect string DSN instead of DSN object This ensures telemetry envelopes can be properly serialized to JSON while maintaining consistency with how other envelope types handle DSN.
Author
|
Sorry for that. Was too fast and also found out, that my expectations were wrong. Indeed the other envelopes are converted to string. I will come up with a new commit soon. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: Convert DSN to string in telemetry envelope header
Problem
When enabling log ingestion with
config.enable_logs = true, the application crashes with the following error:Root Cause
The
TelemetryEventBuffer#send_itemsmethod incorrectly passes the DSN as a Ruby object instead of a string:When the envelope is serialized in
Transport#serialize_envelope, it callsJSON.generate(envelope.headers), which fails because the DSN is a complex Ruby object (instance ofSentry::DSN), not a primitive JSON-serializable type.The Sentry server expects all envelope header values to be strings, numbers, or booleans - not maps/objects.
Solution
Convert the DSN to a string using
.to_sbefore adding it to the envelope header:Why this approach:
Transport#envelope_from_event(line 122), which usesdsn: @dsn.to_stransport_spec.rbtestsChanges
sentry-ruby/lib/sentry/telemetry_event_buffer.rb: Convert DSN to string with.to_sin envelope headersentry-ruby/spec/sentry/metrics_spec.rb: Fix test to expect string DSN instead of DSN objectTesting
config.enable_logs = trueAffected Versions
This bug affects all versions with the telemetry event buffer, including:
config.enable_logsorconfig.enable_metricsfeaturesRelated Files
sentry-ruby/lib/sentry/telemetry_event_buffer.rb(fixed)sentry-ruby/lib/sentry/transport.rb(reference implementation at line 122)sentry-ruby/spec/sentry/transport_spec.rb(shows DSN as string in tests)