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

Feature Request: Allow Breadcrumb data to accept objects as values #88

Closed
lunim opened this issue Aug 26, 2018 · 10 comments
Closed

Feature Request: Allow Breadcrumb data to accept objects as values #88

lunim opened this issue Aug 26, 2018 · 10 comments
Labels
Question Further information is requested

Comments

@lunim
Copy link

lunim commented Aug 26, 2018

Currently, the breadcrumb accepts additional data as a dictionary of <string,string>. Allowing objects as values would remove the need for end-users to serialize. Currently, the SentryEvent extras accepts <string,object>, updating breadcrumb to be the same would be useful.

@bruno-garcia bruno-garcia added the Feature New feature or request label Aug 27, 2018
@bruno-garcia
Copy link
Member

Currently Sentry doesn't really support a serialized object in its data bag.
Serializing an object shows up like this:

image

I think this could be a feature request opened on Sentry and if implemented, change the SDKs which is a coordinated thing now as align the API.

@bruno-garcia
Copy link
Member

@lunim I'll close this as per my comment above.
If Sentry itself is changed to support breadcrumbs value as objects we can reopen this and implement support in this SDK.

@boukeversteegh
Copy link

It seems that this feature is now implemented:

getsentry/sentry-javascript#623 (comment)

Can we reopen this perhaps?

@bruno-garcia bruno-garcia reopened this Aug 3, 2021
@bruno-garcia
Copy link
Member

It's a bit of an issue to take object because often people put data that can't be serialized by default and we have issues.

Forcing this to be ISerializable will introduce friction, since one would need to implement the interface and get serialization done. To be quite honest at this point I would prefer the caller serializes it.

@boukeversteegh
Copy link

boukeversteegh commented Aug 4, 2021 via email

@bruno-garcia
Copy link
Member

It currently accepts string as a value so it should already work if you're serializing before setting it.

There are different extension method, for example accepting a value tuple:

(string, string)? dataPair = null,

@boukeversteegh
Copy link

I see, thanks for your reply. Looking at it, I have two questions:

  • How do I use this overload? SentrySdk.AddBreadCrumb is the documented approach, but this overload is not available on the SentrySdk static class.
  • In the source code, it seems that it adds a KeyValuePair to the Dictionary<string,string>, so we're not really setting the top-level data object.

Thinking about it more, I suppose that as long as the Sentry UI displays the string values as is, including whitespace and linebreaks, then it doesn't really matter whether or not it understands JSON, since we can include a pretty-printed json string in the breadcrumbs.

I will experiment with this approach.

@bruno-garcia
Copy link
Member

I see, thanks for your reply. Looking at it, I have two questions:

  • How do I use this overload? SentrySdk.AddBreadCrumb is the documented approach, but this overload is not available on the SentrySdk static class.

Those extensions are available when adding directly to the event or to the scope. The latter is what SentrySdk.AddBreadcrumb does. You can see all extension methods through:

SentrySdk.ConfigureScope(s => s.AddBreadcrumb(...)
  • In the source code, it seems that it adds a KeyValuePair to the Dictionary<string,string>, so we're not really setting the top-level data object.

Not sure I follow. What do you mean by the top level data object? The Dictionary<string, string> named data is the field in the breadcrumb object where we can define arbitrary data.
We document this here: https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/

image

The fields at the level of the breadcrumb (ie.: its properties) are typed and you can't add data that's not specified in the documentation above.

Thinking about it more, I suppose that as long as the Sentry UI displays the string values as is, including whitespace and linebreaks, then it doesn't really matter whether or not it understands JSON, since we can include a pretty-printed json string in the breadcrumbs.

I will experiment with this approach.

Sounds good! I hope this works out.

@bruno-garcia bruno-garcia added Question Further information is requested and removed Feature New feature or request labels Aug 25, 2021
@SimonCropp
Copy link
Contributor

@boukeversteegh @lunim looks like @bruno-garcia answered this one.

@boukeversteegh
Copy link

Sorry to keep this open.

My current workaround is to pass a dictionary like this;

Dictionary<string, string> data = new();
data.Add(key, JsonFormatter.Format(message)); // formats a Protobuf message to JSON
SentrySdk.AddBreadcrumb(description, category, null, data);

It's just shown as a string, and the json structure is not handled in any way, but it's workable for small messages. Haven't tested yet with formatted/multiline strings.

Closing this is OK I suppose. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants