WIP fix: Ensure structured logs from an SDK integration has sentry.origin

[alexsohn1126]

Fixes #4560

Problem

According to our SDK specification, we have to set sentry.origin whenever a log is sent from an integration.

Solution

sentry-dotnet has a few integrations that send out logs. This PR affects the following integrations:

• Sentry.Extensions.Logging
• Sentry.Serilog

And affects other integrations that references Sentry.Extensions.Logging, or references it transitively.

Comparison

Before, logs from Sentry.Serilog didn't have sentry.origin:

After, logs from Sentry.Serilog has sentry.origin populated:

[alexsohn1126]

@sentry review

[alexsohn1126]

bugbot review

[cursor]

🚨 Bugbot couldn't run

Something went wrong. Try again by commenting "Cursor review" or "bugbot run", or contact support (requestId: serverGenReqId_8ec61e90-31f9-40d2-9718-56e6535d466d).

[alexsohn1126]

bugbot review

[alexsohn1126]

@sentry review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.47%. Comparing base (bc5c060) to head (baf7c13).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4566      +/-   ##
==========================================
+ Coverage   73.45%   73.47%   +0.01%     
==========================================
  Files         480      482       +2     
  Lines       17589    17683      +94     
  Branches     3469     3495      +26     
==========================================
+ Hits        12920    12992      +72     
- Misses       3785     3800      +15     
- Partials      884      891       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Flash0ver]

I'm not 100 % convinced of the origin names that we use right now, but also don't have a concrete suggestion in store ... if you don't mind, I'll let my mind wander a bit and will get back to you soon.
Also, feedback from other reviewers appreciated. @jamescrosswell what do you think?

[jamescrosswell]

I'm not 100 % convinced of the origin names that we use right now, but also don't have a concrete suggestion in store ... if you don't mind, I'll let my mind wander a bit and will get back to you soon.
Also, feedback from other reviewers appreciated. @jamescrosswell what do you think?

From memory, we started setting the sentry.origin initially to support tracing... so the naming conventions for these were made with that in mind. It's even titled Trace Origin in the dev docs.

• The first/type part of the origin is obvious (either manual or auto).
• The second/category part is somewhat subjective (e.g. db or ui) and we try to match whatever the other SDKs are doing here... but some judgement required on our part since the specific databases etc. we're integrating with will often be .NET specific
• The third part is the integration name. Ideally this would match the name we used for the integration in our source... so for example in the Sentry.EntityFramework integration we used the origin auto.db.entity_framework. The challenge is that our NuGet packages are optimised to minimise dependencies, so where we can integrate from the Sentry.nupkg without any extra dependencies, we do... which I think is why we ended up with stuff like auto.graphql (in the base Sentry NuGet).
• I don't think we use the fourth part / integration-part anywhere in the .NET SDK...

[jamescrosswell]

Are these always supposed to be auto for structured logs? If the SDK user does ILogger.Log(...) then shouldn't this be manual?

[alexsohn1126]

Yep, good point. The docs indeed say we should set the type to manual when the user manually sends a log... I'll fix it!

[alexsohn1126]

Wait, upon more thinking, this has more nuance than I thought. Wouldn't the SDK user pretty much always have to call ILogger.Log(...) to log something? Does that mean that sentry.origin should always be manual.[...]?

The documentation linked above states that:

traces
Required. Can be either manual or auto. manual indicates that the user created the trace or span, auto indicates that the SDK or some integration created it.

It doesn't mention logs, but I assume if a user creates a log, then we put manual. So is it always manual, since logs cannot be created without some kind of user code calling ILogger.Log(...)?

@Flash0ver Do you have any input for this?

[alexsohn1126]

This is what the docs say for SDK Integration Attributes:

If a log is generated by an SDK integration, the SDK should also set the sentry.origin attribute, as per the Trace Origin documentation. It is assumed that logs without a sentry.origin attribute are manually created by the user.
{
"sentry.origin": "auto.db.graphql"
}

So maybe we differentiate when a user calls ILogger.Log(...) vs. ILogger.LogInformation(...)?

[jamescrosswell]

@alexsohn1126 sorry I might have sent you on a goose chase there.

Speaking with @Flash0ver about this it might be that:

• If the logs are coming via one of our integrations (e.g. Sentry.Extensions.Logging or Sentry.Serilog) then you'd set this to auto (since we automatically converted a serilog log or an ILogger log to a Sentry Structured one in that case).
• However if an SDK user used the Sentry API to create a log directly then that would be manual

@Flash0ver were you able to confirm this as the desired behaviour?

[alexsohn1126]

That makes things clear! Thanks 👍

[Flash0ver]

clarified with @AbhiPrasad 🥔

• the SDK-Logger (via SentrySdk.Logger.Log*) never sends sentry.origin ... the absence of sentry.origin indicates the Log was captured through the Logger of the respective SDK (sentry.sdk.name)
• all integrations always send auto. .., regardless of the Log-Message being created through the integration from user-code or Application-code/Library-code

[alexsohn1126]

Whoops, forgot to mention that we will be consolidating the category for sentry.origin across other Sentry SDKs! Once we have decided on what category, I'll mark this PR as ready..