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

Event timestamp serialization from JSON #3108

Open
arend-melissant-tnt opened this issue Feb 2, 2024 · 11 comments
Open

Event timestamp serialization from JSON #3108

arend-melissant-tnt opened this issue Feb 2, 2024 · 11 comments
Labels
Good First Issue Good for newcomers

Comments

@arend-melissant-tnt
Copy link

Package

Sentry

.NET Flavor

.NET

.NET Version

8.0

OS

Any (not platform specific)

SDK Version

4.0.0

Self-Hosted Sentry Version

No response

Steps to Reproduce

read envelope from incoming message (using sentry-cli send-event)
deserialize envelope using var envelope = await Envelope.DeserializeAsync(stream);

this will throw an exception: The requested operation requires an element of type 'String', but the target element has type 'Number'.

this is an the "timestamp" element of the event payload

Expected Result

timestamp should deserialize with either a string or number (per SDK)

Actual Result

Exception thrown

@bruno-garcia
Copy link
Member

read envelope from incoming message (using sentry-cli send-event)
deserialize envelope using var envelope = await Envelope.DeserializeAsync(stream);

There might be something to fix here but to to clarify: An envelope and an event are different things. Envelopes contain items, and one of those could be an event.

What are you trying to accomplish? we might be able to help you differently

@arend-melissant-tnt
Copy link
Author

Hi,

I'm just trying to deserialize an complete received envelope, which fails if it contains an event with a timestamp formatted as a number. When te timestamp is formatted as a string everything works as expected. I do see in the code (or so it seems) that timestamps as numbers are not supported, but that should be supported as even the sentry-cli sends such events.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 3, 2024
@bitsandfoxes
Copy link
Contributor

Happy to help but I'm not quite following how to reproduce the issue you're running into:
You're sending an event via sentry-cli but how and why does that generated envelope end up in the .NET SDK? The stream is just the deserialized envelope you're picking up from somewhere?

@arend-melissant-tnt
Copy link
Author

I'm forwarding messages to Sentry, that's how I get the messages. I'll create a small test for you to see what's happening

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 5, 2024
@arend-melissant-tnt
Copy link
Author

arend-melissant-tnt commented Feb 5, 2024

DeserialisationTests.txt

this is a simple unit test to recreate the issue

see https://develop.sentry.dev/sdk/event-payloads/ where it is explained that the timestamp can be either a string or a number

@bitsandfoxes
Copy link
Contributor

I see. Yeah, we rely on GetDateTimeOffset() in System.Text.Json.JsonElement and that strictly expects a string to parse.
I guess we could check the valuetype when parsing from json to event?

@getsantry getsantry bot removed the status in GitHub Issues with 👀 2 Feb 6, 2024
@bitsandfoxes bitsandfoxes added Feature New feature or request Good First Issue Good for newcomers and removed Platform: .NET labels Feb 29, 2024
@bitsandfoxes
Copy link
Contributor

I think we're looking for something like

json.TryGetProperty("timestamp", out var timestampElement);
DateTimeOffset? timestamp = null;
if (timestampElement.ValueKind == JsonValueKind.String)
{
    timestamp = timestampElement.GetDateTimeOffset();
}
else if (timestampElement.ValueKind == JsonValueKind.Number)
{
    timestamp = DateTimeOffset.FromUnixTimeSeconds((long)timestampElement.GetDouble());
}

instead of

var timestamp = json.GetPropertyOrNull("timestamp")?.GetDateTimeOffset();

@arend-melissant-tnt, would you be willing to open a PR for this?

@bitsandfoxes bitsandfoxes changed the title timestamp not deserialized when it is sent as a number Event timestamp serialization from JSON Feb 29, 2024
@bitsandfoxes bitsandfoxes removed the Feature New feature or request label Mar 1, 2024
@Tuytje
Copy link

Tuytje commented Feb 28, 2025

I'm having this issue in the latest version, any idea when this will be fixed?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Feb 28, 2025
@aritchie
Copy link
Collaborator

@Tuytje #3958 <= this PR contains other features, but also a fix for this particular issue

@aritchie
Copy link
Collaborator

Referencing #2102 - close this issue with that issue

@albyrock87
Copy link

@aritchie if it helps this is the involved JSON

{
  "timestamp": "2025-03-01T11:01:48.534Z",
  "type": "system",
  "data": {
    "upload_bandwidth": 150560,
    "action": "NETWORK_CAPABILITIES_CHANGED",
    "vpn_active": false,
    "signal_strength": -60,
    "network_type": "wifi",
    "download_bandwidth": 25236
  },
  "category": "network.event",
  "level": "info"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good for newcomers
Projects
Status: No status
Status: No status
Status: No status
Development

No branches or pull requests

6 participants