Skip to content

fix(serilog): logs from application namespaces beginning with "Sentry" are discarded - #5456

Merged
jamescrosswell merged 2 commits into
mainfrom
serilog-context-filter-5265
Aug 3, 2026
Merged

fix(serilog): logs from application namespaces beginning with "Sentry" are discarded#5456
jamescrosswell merged 2 commits into
mainfrom
serilog-context-filter-5265

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #5265

Summary

SentrySink discarded any Serilog event whose SourceContext started with "Sentry.". That heuristic dates back to 2019 ("don't log self log") and is meant to stop the SDK's own diagnostics looping back into Sentry — but it also matches application code. An app logging under, say, Sentry.Samples.AspNetCore.Serilog.Program had its log events dropped before anything was created: no event, no breadcrumb, no structured log, and therefore no event processors run. That last symptom is how #5265 was reported, but ISentryEventProcessor was never the problem.

Our own samples/Sentry.Samples.AspNetCore.Serilog uses that namespace, so the sample demonstrating the ILogger path was silently broken — which is where the reporter picked it up.

The filter can't simply be removed. MelDiagnosticLogger routes SDK diagnostics through ILogger<ISentryClient> whenever Debug is enabled (ApplicationBuilderExtensions.cs), and with Serilog as the MEL backend that's a genuine feedback loop. The isReentrant guard in the sink doesn't cover it, because the transport logs from the background worker thread.

So the check now matches the namespace roots the SDK actually owns, rather than a bare "Sentry." prefix.

Notes for review

  • The root list is generated, not hand-maintained. A GenerateSentrySdkNamespaces target in Sentry.csproj reads the package list out of .craft.yml at build time and emits SentrySdkNamespaces.g.cs. No reflection (AOT/trimming-safe) and no list to keep in sync as packages are added. .craft.yml's line order is fixed, so output is deterministic. The target hard-errors rather than emitting an empty list, since an empty list would silently restore the loop risk.
  • The core Sentry package can't be a prefix root. All of its types live under Sentry. — and so does user code — so treating it as a prefix would reproduce the original bug. Core has no logging dependency of its own, and the only category it ever logs under is Sentry.ISentryClient (from MelDiagnosticLogger), so that plus a bare Sentry are matched exactly. This is the one hardcoded string in the change; it's commented next to the thing that produces it.
  • Sentry.Extensions.Logging is included deliberately. SentryLogger.IsFromSentry() carried the same heuristic plus a #if DEBUG carve-out for Sentry.Samples., which only ever helped samples built from source — anyone on the released package was still affected. Both integrations now share one helper. Sentry.NLog and Sentry.Log4Net have never had this filter at all; that asymmetry is left as-is here.
  • .craft.yml gains Sentry.Maui.CommunityToolkit.Mvvm, which is packable but was missing from the registry list. Worth a second pair of eyes: it's a release-config change riding along with a bug fix, and it means the release registry starts tracking that package... main reason for including it in this PR is that we're using craft.yml to generate the list of namespaces that we match against for log filtering.

…ing with "Sentry"

The Serilog sink dropped any log event whose SourceContext started with "Sentry.",
a heuristic meant to keep the SDK's own diagnostics from looping back into Sentry.
It also matched application code, so an app logging under e.g.
"Sentry.Samples.AspNetCore.Serilog.Program" had its events silently discarded -
no event, no breadcrumb, no structured log, and no event processors run. Our own
ASP.NET Core Serilog sample is written in exactly that namespace.

The filter is still needed: MelDiagnosticLogger routes SDK diagnostics through
ILogger<ISentryClient> when Debug is enabled, which Serilog would otherwise feed
straight back in. So instead of matching a bare prefix, match the namespace roots
the SDK actually owns. Those are generated at build time from the package list in
.craft.yml, so the list can't go stale as packages are added.

The core "Sentry" package can't be a prefix root - all its types live under
"Sentry.", as does user code - so it is matched exactly, along with the one
category core ever logs under.

Sentry.Extensions.Logging shared the same heuristic, including a #if DEBUG
carve-out for "Sentry.Samples." that only ever helped samples built from source.
Both integrations now use the same helper.

Also adds the missing Sentry.Maui.CommunityToolkit.Mvvm entry to .craft.yml.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jamescrosswell jamescrosswell changed the title fix(serilog): stop discarding logs from application namespaces beginning with "Sentry" fix(serilog): logs from application namespaces beginning with "Sentry" are discarded Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.70%. Comparing base (8d4d3f6) to head (252fe48).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5456   +/-   ##
=======================================
  Coverage   74.69%   74.70%           
=======================================
  Files         512      513    +1     
  Lines       18722    18729    +7     
  Branches     3660     3663    +3     
=======================================
+ Hits        13985    13992    +7     
  Misses       3865     3865           
  Partials      872      872           

☔ View full report in Codecov by Harness.
📢 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.

Comment thread test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs Outdated
Comment thread test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs Outdated
Comment thread test/Sentry.Serilog.Tests/SentrySinkTests.cs Outdated
Comment thread test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs Outdated
Comment thread test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs Outdated
.
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review July 30, 2026 04:14
@github-actions github-actions Bot added the risk: high PR risk score: high label Jul 30, 2026

@alexander-alderman-webb alexander-alderman-webb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The src/Sentry/Internal/SentrySdkNamespaces.cs logic looks sound to me.

@jamescrosswell
jamescrosswell merged commit e669940 into main Aug 3, 2026
53 checks passed
@jamescrosswell
jamescrosswell deleted the serilog-context-filter-5265 branch August 3, 2026 22:08
Lms24 pushed a commit to getsentry/sentry-release-registry that referenced this pull request Aug 18, 2026
### Context

- Same failure mode as #223 (`Sentry.Hangfire`) and #221
(`Sentry.Extensions.AI`)
- Follow-up to getsentry/sentry-dotnet#5456
- Tracking issue: getsentry/sentry-dotnet#5495

### Summary

`Sentry.Maui.CommunityToolkit.Mvvm` has shipped on NuGet since 5.8.0 but
was never added here. getsentry/sentry-dotnet#5456 added it to
sentry-dotnet's `.craft.yml` registry target, and the 6.9.0 publish then
aborted ([run
32010411662](https://github.com/getsentry/publish/actions/runs/32010411662)):

```
[error] "name" is required for new package "nuget:Sentry.Maui.CommunityToolkit.Mvvm".
        Add `name` to the registry target config in your .craft.yml.
```

Craft treats a package as new only when its `latest.json` is missing, so
seeding the directory here makes the bare `.craft.yml` key work exactly
as it does for the other 18 sentry-dotnet packages.

The failure isn't scoped to this one package — `updateVersionInRegistry`
throws before craft commits anything, so the whole registry target rolls
back. Every sentry-dotnet package is currently still pinned at 6.8.0
here, two releases into 6.9.0 being live on NuGet.

### Notes for review

- **Seeded at 6.8.0, not 6.9.0, deliberately.** Craft aborts with
`Version file for "6.9.0" already exists. Aborting.` if the version
being published is already present — which would roll the target back
again and leave all 19 packages stuck at 6.8.0. Seeding the previous
version (which the package did ship) lets the retry create 6.9.0 and
re-point the symlinks. #223 did the same, seeding
`6.0.0-rc.2-prerelease`.
- `canonical` has to match the `.craft.yml` key exactly or craft errors
on the consistency check in `getUpdatedManifest`.
- `created_at` is the value craft wrote for the sibling packages in the
6.8.0 release, since it's the same release event.
- Symlinks generated with `bin/sync-links`, not by hand. I also re-ran
the full `make sync-all-links` (via `xargs`, as the bare `find` blows
the argument limit on macOS) and confirmed it reproduces the committed
tree exactly, so the CI sync check should be clean.
- No `sdks/sentry.dotnet.maui.communitytoolkit.mvvm` symlink. Every
other dotnet package has one, but this package doesn't emit its own
`sdk_info.name` — its events go out as `sentry.dotnet.maui` — so the
entry would name something nothing reports. Happy to add it if
uniformity is preferred.

### Relationship to getsentry/sentry-dotnet#5496

#5496 approaches the same failure from the other side: it gives
sentry-dotnet's `.craft.yml` the `name`/`packageUrl`/`mainDocsUrl` that
craft needs to create manifests itself — for *every* package, not just
this one, so the next new package doesn't repeat this.

The two are complementary rather than exclusive. This PR unblocks the
6.9.0 retry now; #5496 stops it happening again. If both land, this
package's `.craft.yml` entry is simply belt-and-braces — craft applies
the config over the manifest on every release either way.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 18, 2026
jamescrosswell added a commit that referenced this pull request Aug 24, 2026
… packages (#5496)

* ci(craft): give Sentry.Maui.CommunityToolkit.Mvvm the registry metadata craft needs

The 6.9.0 publish pushed to NuGet and cut the GitHub release, then aborted on the
registry target:

    "name" is required for new package "nuget:Sentry.Maui.CommunityToolkit.Mvvm".
    Add `name` to the registry target config in your .craft.yml.

#5456 added the package to the registry target as a bare key. That works for the
other entries because they already exist in sentry-release-registry - craft reads
their latest.json and carries it forward. This package has shipped on NuGet since
5.8.0 but was never added to the registry, so craft has to create its manifest,
and for that it needs `name`, plus `packageUrl`/`mainDocsUrl` for the manifest to
match the shape of its neighbours.

The failure isn't scoped to the one package: updateVersionInRegistry throws before
craft commits, so the whole registry target rolls back and every sentry-dotnet
package is still pinned at 6.8.0 in the registry.

The bare `nuget:<package>:` line is left intact so the GenerateSentrySdkNamespaces
target in Sentry.csproj still picks the package up - verified by running the target
and diffing SentrySdkNamespaces.g.cs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Apply suggestion from @jamescrosswell

* ci(craft): give every registry entry the metadata craft needs

Extends the previous commit to the whole list rather than just the one package
that happened to break 6.9.0.

Craft only reads name/packageUrl/mainDocsUrl for a package it hasn't published
before - for everything already in the release registry it carries the existing
manifest forward, so a bare key works. The catch is that a bare key keeps working
right up until the release that first ships a new package, and then fails the
entire registry target, not just that package. That's caught us on nearly every
new package: Sentry.Extensions.AI, Sentry.Hangfire, and now
Sentry.Maui.CommunityToolkit.Mvvm.

Filling the metadata in everywhere makes the working pattern the default: a new
package is added by copying the block above it, and craft creates the registry
entry on its own.

Values are copied verbatim from the manifests in sentry-release-registry, not
hand-written. Craft writes these fields over the manifest on every release, so a
typo here would silently rewrite published metadata - including the stale-looking
docs URLs, which are deliberately left exactly as they are rather than quietly
corrected in a CI commit.

Verified: the generated SentrySdkNamespaces.g.cs is byte-identical after a forced
regeneration, and every value round-trips against the registry manifests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ci(craft): point registry docs URLs at pages that exist

Now that craft writes main_docs_url back to the release registry on every
release, these values are published metadata rather than dead config, so the
stale ones are worth correcting instead of carrying forward.

Two were outright 404s:

  Sentry.AspNet                 /platforms/dotnet/aspnet      -> guides/aspnet/
  Sentry.Google.Cloud.Functions /platforms/dotnet/gcp-functions
                                                -> guides/google-cloud-functions/

Four pointed at URLs that only worked via redirect, now canonicalised
(aspnetcore, entityframework, microsoft-extensions-logging, and OpenTelemetry's
performance/ -> tracing/ move), and six sat on the generic platform root despite
having their own page: Log4Net, NLog, Serilog, DiagnosticSource (the
automatic-instrumentation page is specifically about Sentry.DiagnosticSource),
Blazor.WebAssembly (was pointing at the ASP.NET Core guide), and AspNetCore.Grpc.

Grpc is the one that stays imprecise - there is no .NET gRPC page, so it points
at the ASP.NET Core guide it extends rather than the platform root.

Every URL in the file, docs and nuget alike, was checked to return 200 *and* be
the canonical target, so we don't bake in another redirect that later 404s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ci(craft): point Sentry.OpenTelemetry.Exporter at its own docs page

Review catch on #5496. The Exporter was inheriting Sentry.OpenTelemetry's docs
URL - a value carried over from the registry, not one this PR introduced, so it
slipped through when the sibling entry was canonicalised.

There is a dedicated "OpenTelemetry (OTLP)" page: it names
Sentry.OpenTelemetry.Exporter nine times and nothing else, and it's what the
README's documentation badge for the package already links to. The plain
opentelemetry/ page is about Sentry.OpenTelemetry, mentioning the exporter only
in passing.

Cross-checking the rest of the file against the README badges turned up one more:
Sentry.DiagnosticSource has an anchor on the shared automatic-instrumentation
page, so it now points at #diagnosticsource-integration rather than the top of a
page that also covers other integrations.

The README's own badges for Sentry.OpenTelemetry and Sentry.DiagnosticSource
still use the pre-rename performance/ paths. Those redirect rather than 404, and
the README is outside this PR's scope, so they're left for a follow-up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: update the two README badges left on the pre-rename performance/ path

Sentry.DiagnosticSource and Sentry.OpenTelemetry are the only README
documentation badges still pointing at /platforms/dotnet/performance/..., which
301s to /platforms/dotnet/tracing/... Nothing was broken - the badges land on the
right page today, and the DiagnosticSource anchor survives since fragments are
client-side - but a redirect that works today is exactly what later becomes a
404, which is how /platforms/dotnet/aspnet and /gcp-functions ended up dead.

Both README badges now match what .craft.yml publishes to the release registry
for the same packages.

The remaining README docs links that redirect do so only for a missing trailing
slash, which is a different (and cosmetic) thing - left alone here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentry.Serilog Does Not Use ISentryEventProcessor When Microsoft.Extensions.Logging.ILogger Is Used

2 participants