Development: Adjust Sentry trace sampling to reduce load on Sentry
#11666
Conversation
github-actions
bot
added
server
WalkthroughCustom Sentry traces samplers are introduced in both Java and TypeScript configurations to selectively sample transactions based on request properties and environment. Production sampling rates are reduced from 0.20 to 0.05, with specific endpoints excluded or downsampled. Environment blank-value handling is also standardized. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/webapp/app/core/sentry/sentry.error-handler.ts (1)
36-37: Align with TypeScript and style guidelines for minor improvementsThe sampler implementation correctly uses stable Sentry v10.25.0 APIs—
samplingContext.nameandinheritOrSampleWithare both documented and available. Two optional style refinements align with project guidelines:
- Change
let defaultSampleRatetoconst(variable is never reassigned)- Use single quotes instead of double quotes for string literals
- let defaultSampleRate: number = this.environment !== PROFILE_PROD ? 1.0 : 0.05; + const defaultSampleRate: number = this.environment !== PROFILE_PROD ? 1.0 : 0.05; tracesSampler: (samplingContext) => { const { name, inheritOrSampleWith } = samplingContext; // Sample none of the time transactions - if (name.includes("api/core/public/time")) { + if (name.includes('api/core/public/time')) { return 0.0; } // Sample less of the iris status transactions - if (name.includes("api/iris/status")) { + if (name.includes('api/iris/status')) { return 0.001; } // Fall back to default sample rate return inheritOrSampleWith(defaultSampleRate); },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java(6 hunks)src/main/webapp/app/core/sentry/sentry.error-handler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Files:
src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/core/sentry/sentry.error-handler.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: otoriphoenix
Repo: ls1intum/Artemis PR: 11388
File: src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java:92-102
Timestamp: 2025-10-24T22:55:10.778Z
Learning: In src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java, the substring matching using env.contains("test") and env.contains("staging") in the getTracesSampleRate() method is intentional. The project has multiple testing and staging environments whose names contain these strings, and they want to apply a common sample rate (1.0) to all such environments.
Learnt from: otoriphoenix
Repo: ls1intum/Artemis PR: 11388
File: docker/artemis/config/postgres.env:5-5
Timestamp: 2025-09-30T10:31:58.026Z
Learning: When using Sentry JDBC instrumentation with P6Spy, only a minimal spy.properties configuration is required: `modulelist=com.p6spy.engine.spy.P6SpyFactory`. This is the recommended configuration per Sentry's official documentation and prevents creating unnecessary spy.log files while enabling Sentry's SentryJdbcEventListener to capture database query spans.
📚 Learning: 2025-10-24T22:55:10.778Z
Learnt from: otoriphoenix
Repo: ls1intum/Artemis PR: 11388
File: src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java:92-102
Timestamp: 2025-10-24T22:55:10.778Z
Learning: In src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java, the substring matching using env.contains("test") and env.contains("staging") in the getTracesSampleRate() method is intentional. The project has multiple testing and staging environments whose names contain these strings, and they want to apply a common sample rate (1.0) to all such environments.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.javasrc/main/webapp/app/core/sentry/sentry.error-handler.ts
📚 Learning: 2025-09-30T10:31:58.026Z
Learnt from: otoriphoenix
Repo: ls1intum/Artemis PR: 11388
File: docker/artemis/config/postgres.env:5-5
Timestamp: 2025-09-30T10:31:58.026Z
Learning: When using Sentry JDBC instrumentation with P6Spy, only a minimal spy.properties configuration is required: `modulelist=com.p6spy.engine.spy.P6SpyFactory`. This is the recommended configuration per Sentry's official documentation and prevents creating unnecessary spy.log files while enabling Sentry's SentryJdbcEventListener to capture database query spans.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java
🧬 Code graph analysis (1)
src/main/webapp/app/core/sentry/sentry.error-handler.ts (1)
src/main/webapp/app/app.constants.ts (2)
PROFILE_PROD(73-73)VERSION(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: bean-instantiation-check
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: server-tests
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java (1)
108-111: Verify actualsentry.environmentvalues before applying this changeThe learning from previous work confirms that
contains()was intentional to match multiple environments whose names contain "test" or "staging" at any position (e.g., "my-test-backend", "staging-api"). The current code uses the narrowerstartsWith()check, which would miss such environments.However, the actual impact depends on your deployment's specific environment naming patterns. Since configuration files are not accessible in the repository (likely set via environment variables or deployment-specific configs), you need to:
- Verify the actual
sentry.environmentvalues across all test and staging deployments- Confirm whether narrowing to
startsWith()is causing any environments to fall back to0.0sampling unintentionallyIf your environments follow patterns like
test-*,*-test,staging-*, or*-staging, then restoringcontains()aligns with the documented intent. If all environments strictly start with these prefixes, thenstartsWith()is sufficient.
| import jakarta.servlet.http.HttpServletRequest; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Guard against missing/incorrect HttpServletRequest in tracesSampler
samplingContext.getCustomSamplingContext().get("request") is blindly cast and dereferenced. For non-HTTP transactions or if the custom context is not populated as expected, this will throw a NullPointerException (or ClassCastException) inside the sampler and can break tracing for all such transactions.
Consider defensively handling the absence or wrong type of request and falling back to the parent sampling decision or the environment-based default:
- SentryOptions.TracesSamplerCallback tracesSampler = samplingContext -> {
- HttpServletRequest request = (HttpServletRequest) samplingContext.getCustomSamplingContext().get("request");
- String url = request.getRequestURI();
- String method = request.getMethod();
+ SentryOptions.TracesSamplerCallback tracesSampler = samplingContext -> {
+ Object requestObject = samplingContext.getCustomSamplingContext().get("request");
+ if (!(requestObject instanceof HttpServletRequest request)) {
+ // Non-HTTP transaction: respect parent decision or fall back to env-based default
+ Boolean parentSampled = samplingContext.getTransactionContext().getParentSampled();
+ if (parentSampled != null) {
+ return parentSampled ? 1.0 : 0.0;
+ }
+ return getTracesSampleRate();
+ }
+
+ String url = request.getRequestURI();
+ String method = request.getMethod();
- if (method.equals("HEAD")) {
+ if ("HEAD".equals(method)) {
// We're not interested in HEAD requests, so we just drop them (113 transactions per minute)
return 0.0;
}This keeps the new filtering behavior while making the sampler robust for all transaction types.
Please double-check Sentry Java’s TracesSamplerCallback documentation for recommended use of customSamplingContext and parentSampled in your version.
Also applies to: 16-17, 57-90
🌐 Web query:
In Sentry Java, what guarantees are there about `customSamplingContext` and the presence of an HTTP `request` key in `TracesSamplerCallback`, and how is `parentSampled` intended to be used?
💡 Result:
Short answer:
-
customSamplingContext: The TracesSamplerCallback receives a SamplingContext that may include a CustomSamplingContext if one was passed to startTransaction; but its contents are not guaranteed by the SDK — it only contains what the caller or the specific integration put there. Server/framework integrations often put the incoming HTTP request under a "request" (or similar) key, but that is an integration-level convenience, not a universal language-level guarantee. [1][2][3]
-
presence of HTTP request key: There is no absolute guarantee the SamplingContext will contain an HTTP request key. Docs say server frameworks "may" include the request; example Spring integration shows reading a HttpServletRequest from the custom sampling context if present. Do not rely on it unless your framework/integration documents that it adds it or you explicitly pass it when starting the transaction. [2][3]
-
parentSampled usage: parentSampled is exposed on the TransactionContext (SamplingContext.transactionContext.parentSampled) and represents the sampling decision inherited from an upstream/parent transaction. The recommended usage is to respect (return) that decision in your tracesSampler (e.g., return 1.0 if parentSampled true, 0.0 if false) to avoid breaking distributed traces. You may override it, but doing so can break trace continuity. The tracesSampler may also return null (let SDK fall back) or a sample rate (0.0–1.0). [1][4]
Sources:
[1] Sentry sampling docs (sampling context & parent sampling precedence).
[2] Sentry Java/Spring tracing docs (example showing request in customSamplingContext).
[3] io.sentry.SamplingContext Javadoc (API: getCustomSamplingContext, getTransactionContext).
[4] Sentry Java sampling docs/example showing returning parentSampled in tracesSampler.
Now let me verify the actual code in the repository:
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect SentryConfiguration.java to verify the NPE vulnerability
fd SentryConfiguration.java --type f -x cat {} \; | head -150
Length of output: 5559
Guard against missing/incorrect HttpServletRequest in tracesSampler to prevent NullPointerException
Lines 57–59 cast and immediately dereference samplingContext.getCustomSamplingContext().get("request") without any null or type checks. The web search confirms that there is no guarantee the request key exists in the custom sampling context—it is added only if the Spring framework integration explicitly populates it. For non-HTTP transactions or if the context is not populated, this will throw a NullPointerException and crash the sampler, potentially breaking tracing for all such transactions.
Defensively handle the absence or wrong type of request by checking with instanceof and falling back to the parent sampling decision or the environment-based default:
SentryOptions.TracesSamplerCallback tracesSampler = samplingContext -> {
- HttpServletRequest request = (HttpServletRequest) samplingContext.getCustomSamplingContext().get("request");
+ Object requestObject = samplingContext.getCustomSamplingContext().get("request");
+ if (!(requestObject instanceof HttpServletRequest request)) {
+ // Non-HTTP transaction: respect parent decision or fall back to env-based default
+ Boolean parentSampled = samplingContext.getTransactionContext().getParentSampled();
+ if (parentSampled != null) {
+ return parentSampled ? 1.0 : 0.0;
+ }
+ return getTracesSampleRate();
+ }
+
String url = request.getRequestURI();
String method = request.getMethod();
- if (method.equals("HEAD")) {
+ if ("HEAD".equals(method)) {This keeps the existing filtering behavior while making the sampler robust for all transaction types.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import jakarta.servlet.http.HttpServletRequest; | |
| SentryOptions.TracesSamplerCallback tracesSampler = samplingContext -> { | |
| Object requestObject = samplingContext.getCustomSamplingContext().get("request"); | |
| if (!(requestObject instanceof HttpServletRequest request)) { | |
| // Non-HTTP transaction: respect parent decision or fall back to env-based default | |
| Boolean parentSampled = samplingContext.getTransactionContext().getParentSampled(); | |
| if (parentSampled != null) { | |
| return parentSampled ? 1.0 : 0.0; | |
| } | |
| return getTracesSampleRate(); | |
| } | |
| String url = request.getRequestURI(); | |
| String method = request.getMethod(); | |
| if ("HEAD".equals(method)) { | |
| return 0.0; | |
| } | |
| // ... remaining sampler logic | |
| }; |
🤖 Prompt for AI Agents
In src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java
around lines 57–59, the tracesSampler dereferences
samplingContext.getCustomSamplingContext().get("request") without null/type
checks which can cause a NullPointerException; update the sampler to first
retrieve the object into a variable, check for null and that it is an instance
of HttpServletRequest using instanceof, and only cast and use it when valid; if
missing or wrong type, fall back to the parent sampling decision or the
environment-based default sampling decision so non-HTTP transactions continue to
be sampled safely.
| // For more fine-grained control over what we want to sample, we define a custom traces sampler. | ||
| // By default, it returns null to instead use the tracesSampleRate (as specified per the Sentry documentation). | ||
| // This filters out noise to enable us to focus on the transactions that actually impact Artemis functionality. |
There was a problem hiding this comment.
🧩 Analysis chain
Align sampler fallback behavior and comments with Sentry’s API
The comment says the sampler “by default … returns null to instead use the tracesSampleRate”, but the implementation never returns null; it always returns a concrete double (including getTracesSampleRate() in the default path). At the same time options.setTracesSampleRate(getTracesSampleRate()); is documented as “configured as a fallback”.
To avoid confusion:
- Either rely on Sentry’s built-in fallback by returning
nullfrom the sampler in the default case and keepingsetTracesSampleRate, or - Drop
setTracesSampleRateentirely and treatgetTracesSampleRate()as your explicit, environment-based default within the sampler (updating the comments accordingly).
Right now the comments suggest the former, while the code effectively does the latter.
- // By default, it returns null to instead use the tracesSampleRate (as specified per the Sentry documentation).
+ // By default, it uses getTracesSampleRate(), mirroring the global tracesSampleRate configuration for root transactions.
...
- options.setTracesSampleRate(getTracesSampleRate()); // configured as a fallback
+ // Optionally keep this if the sampler can return null in the future; otherwise it is unused.
+ options.setTracesSampleRate(getTracesSampleRate());Pick one strategy and make the comments match to prevent future misconfigurations.
Please confirm in your Sentry Java version whether tracesSampleRate is consulted at all when a tracesSampler is provided and the callback never returns null.
Also applies to: 88-90, 97-99
🌐 Web query:
When `tracesSampler` is set in Sentry Java, is `tracesSampleRate` still applied if the sampler always returns a number (never `null`)?
💡 Result:
If you provide a tracesSampler that returns a numeric sampling decision (0.0–1.0), that decision takes precedence and tracesSampleRate is not used. TracesSampler overrides tracesSampleRate; tracesSampleRate is only used when no tracesSampler (and no parent sampling decision) is present. [1][2][3]
Sources:
- Sentry Java options docs (tracesSampleRate / tracesSampler). [1]
- Sentry Log4j2 tracing docs (tracesSampler takes precedence). [2]
- Sentry unified tracing guidelines (behaviour and precedence). [3]
Align sampler fallback behavior and comments with Sentry's API
Sentry Java's tracesSampler overrides tracesSampleRate when it returns a numeric value. Since your implementation always returns a concrete double (never null), the setTracesSampleRate() call is unused, and the comments are misleading.
Update the comments to reflect the actual behavior:
- The sampler does not fall back to
tracesSampleRate; it always applies its own decision viagetTracesSampleRate(). - Remove or clarify the misleading reference to Sentry's fallback behavior.
- Optionally document why
setTracesSampleRate()is retained if there are plans for future null returns.
Example:
- // By default, it returns null to instead use the tracesSampleRate (as specified per the Sentry documentation).
+ // By default, it uses getTracesSampleRate() as the sampling rate; tracesSampler overrides tracesSampleRate.Also applies to: 88-90, 97-99
🤖 Prompt for AI Agents
In src/main/java/de/tum/cit/aet/artemis/core/config/SentryConfiguration.java
around lines 57-59 (also apply same clarification to 88-90 and 97-99): the
comments state the traces sampler will return null to fall back to
tracesSampleRate, but the implementation always returns a concrete double so
tracesSampleRate is never used; update the comments to state that the custom
traces sampler always makes the sampling decision (overriding tracesSampleRate),
remove or reword the misleading fallback reference, and optionally add a short
note explaining why setTracesSampleRate() remains in code (e.g., kept for
possible future null-return behavior) if you want to retain it.
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
Checklist
General
Server
Client
Motivation and Context
Currently, a lot of traces collected by the Sentry SDK are noise. Certain endpoints are not important to trace to evaluate the functionality of Artemis. This PR implements a more dynamic sampling approach to filter out noise and make sure the data within Sentry stays relevant and legible.
Description
I implemented custom sampling criteria in both client and server code. The custom sampling filters out certain noisy transactions and reduces the sample rate of some others.
Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
As the changes are rather small, I did not generate a coverage report. If necessary, I can add them.
Summary by CodeRabbit