Skip to content

feat: add sampling support for tracing#3029

Merged
chronark merged 2 commits intomainfrom
otel-namespace
Mar 26, 2025
Merged

feat: add sampling support for tracing#3029
chronark merged 2 commits intomainfrom
otel-namespace

Conversation

@chronark
Copy link
Collaborator

@chronark chronark commented Mar 25, 2025

the default sampling is now 25% but can be changed via
--otel-trace-sampling-rate=0.1 or
UNKEY_OTEL_TRACE_SAMPLING_RATE=0.8

Summary by CodeRabbit

  • New Features

    • Introduced a configurable option that lets users set the OpenTelemetry trace sampling rate between 0.0 (no traces) and 1.0 (all traces), with 0.25 as the default.
    • Added a new command-line flag and associated environment variable, enabling more granular control over telemetry tracing.
  • Documentation

    • Updated the docs with usage examples and clear explanations of how different sampling rates can impact performance and observability.

the default sampling is now 25% but can be changed via
--otel-trace-sampling-rate=0.1 or
UNKEY_OTEL_TRACE_SAMPLING_RATE=0.8
@changeset-bot
Copy link

changeset-bot bot commented Mar 25, 2025

⚠️ No Changeset found

Latest commit: 4dc4bd2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Mar 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
engineering ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 2:58pm
play ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 2:58pm
www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 2:58pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
dashboard ⬜️ Ignored (Inspect) Visit Preview Mar 25, 2025 2:58pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 25, 2025

📝 Walkthrough

Walkthrough

This change introduces a new configuration option for OpenTelemetry trace sampling across both documentation and code. A new property/flag—accepting a float value between 0.0 and 1.0 with a default of 0.25—is added to control trace sampling. The configuration updates span documentation, the API configuration struct, command-line flags, and the telemetry initialization logic for Grafana. The sampler setup now selects between always, never, or parent-based strategies based on this sampling rate.

Changes

File(s) Change Summary
apps/.../config.mdx Added new configuration property --otel-trace-sampling-rate/UNKEY_OTEL_TRACE_SAMPLING_RATE (float64, default 0.25) with detailed documentation on its usage and expected values (0.0–1.0).
go/apps/api/config.go, go/apps/api/run.go,
go/cmd/api/main.go
Updated configuration structures to include the new OtelTraceSamplingRate field and introduced a new CLI flag. In run.go, the new field (TraceSampleRate) is passed to otel.InitGrafana, integrating this setting into the API initialization workflow.
go/pkg/otel/grafana.go Added a TraceSampleRate field to the configuration struct and modified InitGrafana to select a sampler based on the rate (using always, never, or parent-based strategies). Also added a resource attribute for service namespace.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Config
    participant RunFunction
    participant OtelModule

    User->>CLI: Provide flag (otel-trace-sampling-rate)
    CLI->>Config: Set OtelTraceSamplingRate (default 0.25 if not provided)
    Config->>RunFunction: Construct configuration with TraceSampleRate
    RunFunction->>OtelModule: Call InitGrafana(config)
    OtelModule->>OtelModule: Configure sampler based on TraceSampleRate
    OtelModule-->>RunFunction: Telemetry initialized with appropriate sampler
Loading

Possibly related PRs

Suggested reviewers

  • mcstepp
  • perkinsjr
  • MichaelUnkey
  • ogzhanolguncu

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ef6b02 and 4dc4bd2.

📒 Files selected for processing (5)
  • apps/engineering/content/architecture/services/api/config.mdx (1 hunks)
  • go/apps/api/config.go (1 hunks)
  • go/apps/api/run.go (1 hunks)
  • go/cmd/api/main.go (2 hunks)
  • go/pkg/otel/grafana.go (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Test Go API Local / Test (Shard 2/8)
  • GitHub Check: Test Go API Local / Test (Shard 1/8)
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Build / Build
🔇 Additional comments (10)
go/apps/api/config.go (1)

70-73: Change looks good.

The addition of the OtelTraceSamplingRate field to the Config struct is a clean implementation for storing the OpenTelemetry trace sampling rate. The field is correctly placed within the OpenTelemetry configuration section of the struct, maintaining good organization.

go/apps/api/run.go (1)

46-51: Configuration passed correctly to OpenTelemetry.

The addition of TraceSampleRate field to the otel.Config object is done correctly. The value is properly passed from the main configuration object, ensuring the sampling rate flows through to the telemetry initialization.

go/pkg/otel/grafana.go (4)

47-54: Documentation is clear and comprehensive.

The comment for the TraceSampleRate field is well-written, providing detailed information about the field's purpose, acceptable values, and implications of different settings. This will be very helpful for developers using this configuration option.


93-93: Good addition of service namespace.

The addition of the service namespace attribute to the resource is a good improvement for proper OpenTelemetry semantics, making your traces more compliant with OTel standards.


148-163: Sampling configuration is well-implemented.

The sampler configuration logic properly handles all scenarios:

  • Always sample when rate is 1.0 or higher
  • Never sample when rate is 0.0 or lower
  • Use a parent-based strategy with probabilistic sampling for values in between

The parent-based sampling strategy ensures consistent sampling decisions for related traces, which is a best practice.


169-169: Correct application of sampler to trace provider.

The sampler is correctly passed to the trace provider via the trace.WithSampler(sampler) option, ensuring the configured sampling strategy is actually applied.

apps/engineering/content/architecture/services/api/config.mdx (1)

168-185: Documentation is comprehensive and user-friendly.

The documentation for the new --otel-trace-sampling-rate flag is excellent. It clearly explains:

  • The purpose and valid range (0.0 to 1.0)
  • The meaning of different values (0%, 25%, 100%)
  • The trade-offs between lower and higher sampling rates
  • When the setting takes effect
  • Example usages with different values
  • The corresponding environment variable

This level of detail will help users understand and properly configure trace sampling.

go/cmd/api/main.go (3)

320-342: Well-documented flag for OTEL trace sampling configuration.

The new flag for configuring OpenTelemetry trace sampling rate is well-defined with comprehensive documentation that clearly explains:

  • The purpose and meaning of different sampling rate values
  • Performance and cost implications of different settings
  • The default value (0.25 or 25%)
  • Examples for common use cases

This is a valuable addition that gives users precise control over observability depth vs. performance.


368-370: Config struct properly updated to support the new sampling rate.

The configuration struct has been correctly extended to include both the existing OtelEnabled flag and the new OtelTraceSamplingRate property, properly retrieving the value from the command line parameter.


320-342:

Details

✅ Verification successful

Verify trace sampling implementation in downstream code.

The flag and configuration are well-implemented, but it's important to ensure that the sampling rate is correctly applied in the OpenTelemetry initialization code.

Also applies to: 368-370


🏁 Script executed:

#!/bin/bash
# Check how the sampling rate is used in the OpenTelemetry initialization
rg -A 10 -B 5 "OtelTraceSamplingRate" --type go

Length of output: 2660


Trace Sampling correctly applied in OpenTelemetry initialization

The CLI flag "otel-trace-sampling-rate" is correctly bound and passed downstream. The verification confirmed that:

  • In go/cmd/api/main.go, the flag is read with cmd.Float("otel-trace-sampling-rate").
  • In go/apps/api/config.go, the configuration field OtelTraceSamplingRate is declared.
  • In go/apps/api/run.go, the value is properly applied via TraceSampleRate: cfg.OtelTraceSamplingRate during OpenTelemetry initialization.

No changes are required.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@vercel vercel bot temporarily deployed to Preview – play March 25, 2025 14:54 Inactive
@vercel vercel bot temporarily deployed to Preview – www March 25, 2025 14:54 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard March 25, 2025 14:56 Inactive
@vercel vercel bot temporarily deployed to Preview – play March 25, 2025 14:57 Inactive
@vercel vercel bot temporarily deployed to Preview – www March 25, 2025 14:58 Inactive
@vercel vercel bot temporarily deployed to Preview – engineering March 25, 2025 14:58 Inactive
@chronark chronark requested a review from imeyer March 25, 2025 15:00
@chronark chronark assigned chronark and imeyer and unassigned chronark Mar 25, 2025
Copy link
Contributor

@imeyer imeyer left a comment

Choose a reason for hiding this comment

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

LGTM

small q/nit: does cmd.Float handle conversion properly? If we pass --otel-trace-sampling-rate=ALL, will it fail because that's clearly a string? I RTFC for the cli package and it looks like we're covered. Might want to have a test for the config options at some point. 😄

Copy link
Collaborator Author

that should return an error, cause it’s not parseable as a float

Copy link
Collaborator Author

--otel-trace-sampling-rate=1.0 is the way to go

@chronark chronark merged commit 9ccb949 into main Mar 26, 2025
36 of 37 checks passed
@chronark chronark deleted the otel-namespace branch March 26, 2025 07:35
@chronark
Copy link
Collaborator Author

chronark commented Mar 26, 2025

Incorrect Usage: invalid value "all" for flag -otel-trace-sampling-rate: strconv.ParseFloat: parsing "all": invalid syntax

yeah it fails correctly @imeyer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants