Skip to content

feat: send otel data to otel endpoint#2970

Merged
chronark merged 2 commits intomainfrom
otel-do-what-ian-did-and-hope-it-works
Mar 14, 2025
Merged

feat: send otel data to otel endpoint#2970
chronark merged 2 commits intomainfrom
otel-do-what-ian-did-and-hope-it-works

Conversation

@chronark
Copy link
Collaborator

@chronark chronark commented Mar 14, 2025

Summary by CodeRabbit

  • Documentation

    • Streamlined configuration examples for network ports, database settings, and observability instructions.
  • New Features

    • Introduced a new boolean flag for enabling observability, with enhanced guidance on environment variable usage.
  • Chores

    • Removed legacy telemetry and monitoring services from deployment.
    • Updated logging integrations and dependency versions for simplified initialization and improved performance.
    • Removed unit tests for gossip membership functionality.

@vercel
Copy link

vercel bot commented Mar 14, 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 14, 2025 10:55am
play ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2025 10:55am
www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2025 10:55am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
dashboard ⬜️ Ignored (Inspect) Visit Preview Mar 14, 2025 10:55am

@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2025

⚠️ No Changeset found

Latest commit: 62932b0

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 14, 2025

📝 Walkthrough

Walkthrough

This pull request updates configuration examples and deployment settings while deprecating several telemetry-related services. The changes remove multiple deployment files (for Grafana, Loki, Prometheus, Tempo, etc.) and simplify the Docker Compose setup. The OpenTelemetry integration is reworked by replacing a string-based endpoint configuration with a boolean flag and updating logging imports and initialization calls across many Go packages. Dependency versions and new OpenTelemetry libraries have been added to support the new logging and telemetry mechanisms.

Changes

File(s) Change Summary
apps/engineering/.../config.mdx, go/cmd/api/main.go, go/apps/api/config.go, go/apps/api/run.go Updated CLI examples and property descriptions; removed alternate port and DSN examples; replaced --otel-otlp-endpoint with a new boolean flag --otel and updated telemetry initialization logic.
deployment/docker-compose.yaml, deployment/grafana/{grafana.yaml, provisioning/datasources/datasources.yaml}, deployment/loki/config.yaml, deployment/otel-collector-config.yaml, deployment/prometheus/config.yaml, deployment/tempo/config.yaml Removed telemetry and monitoring services and their configurations, leading to a simplified deployment configuration.
Multiple Go files across go/apps/..., go/internal/..., and go/pkg/... (e.g., routes handlers, cache, circuitbreaker, discovery, membership, ring, etc.) Updated logging imports from github.com/unkeyed/unkey/go/pkg/logging to github.com/unkeyed/unkey/go/pkg/otel/logging and simplified logger initialization in tests and service modules.
go/go.mod Added new OpenTelemetry dependencies and updated versions for several OpenTelemetry and related libraries; removed an unused exporter dependency.
go/pkg/clickhouse/client.go Updated logging import and revised table names in batch processing (e.g., from raw_api_requests_v1 to metrics.raw_api_requests_v1) to reflect a new schema/namespace.
go/pkg/otel/{grafana.go,logging/slog.go} Revised logging configuration: removed the direct Grafana endpoint setting and introduced OTLP log exporter, batch processing with severity filtering, and a global handler setup.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant API_Server
    participant Telemetry
    participant Logger

    User->>CLI: Provide command-line flags (e.g., --otel)
    CLI->>API_Server: Launch API with configuration
    API_Server->>Telemetry: Check OtelEnabled flag
    alt Otel enabled
        Telemetry->>API_Server: Initialize OTLP exporter and logger provider
        API_Server->>Logger: Use OpenTelemetry logging
    else
        API_Server->>Logger: Initialize default logging
    end
    API_Server->>User: Process requests with configured telemetry
Loading

Suggested reviewers

  • mcstepp
  • ogzhanolguncu
  • MichaelUnkey
  • perkinsjr

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 golangci-lint (1.62.2)

Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0)
Failed executing command with error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0)

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 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 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 14, 2025

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

@vercel vercel bot temporarily deployed to Preview – play March 14, 2025 10:46 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard March 14, 2025 10:47 Inactive
@vercel vercel bot temporarily deployed to Preview – engineering March 14, 2025 10:47 Inactive
@vercel vercel bot temporarily deployed to Preview – www March 14, 2025 10:48 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (1)
apps/engineering/content/architecture/services/api/config.mdx (1)

332-332: ⚠️ Potential issue

Update deployment example to use new OpenTelemetry flag.

The AWS ECS Production Cluster example still uses the deprecated --otel-otlp-endpoint flag instead of the new --otel boolean flag.

-  --otel-otlp-endpoint="https://your-grafana-endpoint.com"
+  --otel=true

Additionally, you should add a note about setting the required OpenTelemetry environment variables alongside this example.

🧹 Nitpick comments (8)
go/pkg/zen/README.md (1)

41-41: Update Logging Import to OpenTelemetry

The import on this line has been updated from the old logging package to use the new OpenTelemetry–integrated logger:

"github.com/unkeyed/unkey/go/pkg/otel/logging"

Make sure that any documentation or code samples referring to logging now use this updated path.

Additionally, if markdownlint flags hard tabs on this line (MD010), consider replacing them with spaces for consistency with markdown style guidelines.

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

41-41: Hard tabs
Column: 1

(MD010, no-hard-tabs)

go/pkg/cache/cache.go (1)

14-14: Adopt OpenTelemetry Logging in Cache Module

The logging package import has been modified:

"github.com/unkeyed/unkey/go/pkg/otel/logging"

This ensures that our cache implementation leverages the updated OpenTelemetry logging system. Confirm that log messages and error reporting in the cache continue to work as expected.

go/pkg/circuitbreaker/lib.go (1)

253-257: Ensure Consistent Logging Format

In the circuit breaker code (around lines 253–257), the logger is being called with helpers from the standard library’s log/slog package (e.g., slog.String). To maintain consistency, review whether these helper functions should be replaced by corresponding utilities provided by the new OpenTelemetry logging package or whether a bridging solution is in place.

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

76-91: Consider adding validation for the new OtelEnabled field

While a boolean doesn't typically need validation like a string endpoint would, you might want to consider adding validation logic if there are dependencies when OtelEnabled is true.

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

276-277: Indentation mismatch in usage examples.
Line 276 has no leading space, while line 277 is indented, creating inconsistent formatting.

go/pkg/otel/logging/slog.go (2)

12-24: Global handler pattern might limit config flexibility.
Storing the handler in a global variable and setting it in init() offers simplicity but may complicate advanced usage or testing scenarios, especially if multiple configurations are desired.

Consider providing an initialization function that allows passing custom options, rather than relying on a package-level var.


49-52: New logger function relies on global handler.
This is functional, but leaves little room for injecting mocks or different handlers in unit tests.

Changing the signature to accept a slog.Handler parameter or returning an error if the global handler is uninitialized could allow more flexible usage.

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

128-128: Commented-out insecure flags for local development.
Retaining these lines as comments is fine for reference, but ensure they are not accidentally enabled in production builds.

Also applies to: 153-153

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e2b4bab and 17726e5.

⛔ Files ignored due to path filters (1)
  • go/go.sum is excluded by !**/*.sum
📒 Files selected for processing (44)
  • apps/engineering/content/architecture/services/api/config.mdx (3 hunks)
  • deployment/docker-compose.yaml (0 hunks)
  • deployment/grafana/grafana.yaml (0 hunks)
  • deployment/grafana/provisioning/datasources/datasources.yaml (0 hunks)
  • deployment/loki/config.yaml (0 hunks)
  • deployment/otel-collector-config.yaml (0 hunks)
  • deployment/prometheus/config.yaml (0 hunks)
  • deployment/tempo/config.yaml (0 hunks)
  • go/apps/api/config.go (1 hunks)
  • go/apps/api/routes/services.go (1 hunks)
  • go/apps/api/routes/v2_ratelimit_delete_override/handler.go (1 hunks)
  • go/apps/api/routes/v2_ratelimit_get_override/handler.go (1 hunks)
  • go/apps/api/routes/v2_ratelimit_limit/handler.go (1 hunks)
  • go/apps/api/routes/v2_ratelimit_limit/simulation_test.gox (1 hunks)
  • go/apps/api/routes/v2_ratelimit_set_override/handler.go (1 hunks)
  • go/apps/api/run.go (2 hunks)
  • go/cmd/api/main.go (6 hunks)
  • go/cmd/quotacheck/main.go (2 hunks)
  • go/go.mod (3 hunks)
  • go/internal/services/keys/service.go (1 hunks)
  • go/internal/services/permissions/service.go (1 hunks)
  • go/internal/services/ratelimit/sliding_window.go (1 hunks)
  • go/pkg/cache/cache.go (1 hunks)
  • go/pkg/cache/cache_test.go (1 hunks)
  • go/pkg/cache/simulation_test.go (1 hunks)
  • go/pkg/circuitbreaker/lib.go (1 hunks)
  • go/pkg/clickhouse/client.go (3 hunks)
  • go/pkg/cluster/cluster.go (1 hunks)
  • go/pkg/cluster/cluster_test.go (1 hunks)
  • go/pkg/db/database.go (1 hunks)
  • go/pkg/discovery/redis.go (1 hunks)
  • go/pkg/membership/logger.go (1 hunks)
  • go/pkg/membership/membership_test.go (1 hunks)
  • go/pkg/membership/serf.go (1 hunks)
  • go/pkg/otel/grafana.go (3 hunks)
  • go/pkg/otel/logging/slog.go (2 hunks)
  • go/pkg/ring/ring.go (1 hunks)
  • go/pkg/ring/ring_test.go (1 hunks)
  • go/pkg/sim/simulation.go (2 hunks)
  • go/pkg/testutil/http.go (2 hunks)
  • go/pkg/zen/README.md (1 hunks)
  • go/pkg/zen/middleware_errors.go (1 hunks)
  • go/pkg/zen/middleware_logger.go (1 hunks)
  • go/pkg/zen/server.go (1 hunks)
💤 Files with no reviewable changes (7)
  • deployment/prometheus/config.yaml
  • deployment/tempo/config.yaml
  • deployment/grafana/grafana.yaml
  • deployment/grafana/provisioning/datasources/datasources.yaml
  • deployment/loki/config.yaml
  • deployment/otel-collector-config.yaml
  • deployment/docker-compose.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
go/pkg/zen/README.md

41-41: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🪛 LanguageTool
apps/engineering/content/architecture/services/api/config.mdx

[uncategorized] ~148-~148: Loose punctuation mark.
Context: ...rter: - OTEL_EXPORTER_OTLP_ENDPOINT: The URL of your OpenTelemetry collector...

(UNLIKELY_OPENING_PUNCTUATION)

⏰ Context from checks skipped due to timeout of 90000ms (14)
  • GitHub Check: Test Packages / Test ./packages/api
  • GitHub Check: Test Packages / Test ./packages/hono
  • GitHub Check: Test Packages / Test ./packages/rbac
  • GitHub Check: Test Packages / Test ./packages/nextjs
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Packages / Test ./internal/clickhouse
  • GitHub Check: Test Packages / Test ./internal/keys
  • GitHub Check: Test Packages / Test ./packages/cache
  • GitHub Check: Test Packages / Test ./internal/billing
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Build / Build
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (48)
go/pkg/cache/simulation_test.go (1)

13-13: Consistent Logging Import in Tests

The logging package import has been updated in the test file to:

"github.com/unkeyed/unkey/go/pkg/otel/logging"

This change is in line with our new OpenTelemetry integration. Ensure that the behavior of tests remains unchanged with the new logger.

go/apps/api/routes/v2_ratelimit_set_override/handler.go (1)

15-15: Refactor Logging Import in Handler

The import statement on this line has been updated to use the OpenTelemetry logging package:

"github.com/unkeyed/unkey/go/pkg/otel/logging"

This update aligns the logging mechanism used in the handlers with the new telemetry standards. Verify that any logger initialization or usage in this file is compatible with the updated interface.

go/pkg/circuitbreaker/lib.go (1)

11-11: Switch to OpenTelemetry Logger for Circuit Breaker

The import on this line has been changed to:

"github.com/unkeyed/unkey/go/pkg/otel/logging"

This change is consistent with our overall migration to OpenTelemetry-based logging. Please verify that the logger’s API used in this file remains compatible with any custom logging calls.

go/apps/api/routes/v2_ratelimit_get_override/handler.go (1)

15-15: Logging package updated to use OpenTelemetry

The import path has been updated to use the OpenTelemetry-integrated logging package, which aligns with the PR's objective of sending telemetry data to an OpenTelemetry endpoint.

go/pkg/membership/logger.go (1)

6-6: Logging package updated to use OpenTelemetry

The import has been updated to use the OpenTelemetry-integrated logging package. This change maintains the same interface while enabling OpenTelemetry integration for the memberlist logging adapter.

go/pkg/cluster/cluster_test.go (1)

12-12: Logging package updated to use OpenTelemetry

The import has been updated to use the OpenTelemetry-integrated logging package. The test continues to use the same logging interface, now with OpenTelemetry capabilities.

go/pkg/cache/cache_test.go (1)

13-13: Logging package updated to use OpenTelemetry

The import has been updated to use the OpenTelemetry-integrated logging package. The test file now uses the OpenTelemetry-compatible logging interface while maintaining the same API.

go/pkg/zen/middleware_logger.go (1)

8-8: Update Logging Import to OpenTelemetry Package
The import has been updated from the legacy logging package to the new OpenTelemetry (otel) logging package. This change is consistent with the broader telemetry initiative and aligns with the PR objectives.

go/apps/api/routes/v2_ratelimit_delete_override/handler.go (1)

17-17: Replace Legacy Logging Import with OTEL Logging
The logging package import has been updated to "github.com/unkeyed/unkey/go/pkg/otel/logging". This update ensures that the handler uses the new OpenTelemetry logging interface, which is in line with the overall observability enhancements.

go/pkg/ring/ring.go (1)

14-14: Adopt OpenTelemetry Logging in Ring Module
By replacing the old logging import with "github.com/unkeyed/unkey/go/pkg/otel/logging", the ring module will now log events using the OTEL framework. This is a straightforward change that promotes consistency across the codebase.

go/pkg/zen/middleware_errors.go (1)

9-9: Migrate Error Middleware to OTEL Logging
The middleware error handler now imports the OpenTelemetry logging package. This change aligns error reporting with the new logging infrastructure and ensures better traceability in observability data.

go/apps/api/routes/v2_ratelimit_limit/simulation_test.gox (1)

16-16: Update Test Logging Import to OpenTelemetry
The simulation test suite now uses the OTEL logging package, ensuring that test logs match the new logging framework used throughout the application. This change improves consistency and eases troubleshooting during testing.

go/pkg/db/database.go (1)

9-10: Updated Logging Import:
The old logging package has been replaced with the new OpenTelemetry-based logger (github.com/unkeyed/unkey/go/pkg/otel/logging). This change is consistent with the overall telemetry initiative. Please ensure that any logging configuration or usage downstream leverages the enhanced observability features provided by the OpenTelemetry implementation.

go/apps/api/routes/v2_ratelimit_limit/handler.go (1)

14-14: Updated Logging Import in Handler:
The import change to use github.com/unkeyed/unkey/go/pkg/otel/logging is correctly applied. This reinforces the new logging strategy across API handlers. Verify that all logger invocations produce the expected telemetry output.

go/pkg/ring/ring_test.go (1)

11-11: Consistent Logger Update in Tests:
Updating the logging import to the OpenTelemetry version within the tests ensures consistency with production code. The use of logging.NewNoop() remains appropriate for a test environment.

go/pkg/discovery/redis.go (1)

9-9: Updated Logging Import in Discovery Module:
The replacement of the previous logging package with github.com/unkeyed/unkey/go/pkg/otel/logging here is in keeping with the new telemetry-driven architecture. The logger is used for error reporting (e.g., in the heartbeat function), so confirm that the new logger’s behaviors (such as structured logging and error reporting) meet operational needs.

go/pkg/membership/membership_test.go (1)

12-12: Unified Logging in Membership Tests:
The change to import the OpenTelemetry logging package ensures that the test suite remains consistent with the new telemetry framework implemented across the codebase. This update should help in monitoring and debugging test executions in the future.

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

70-71: LGTM: Simplified OpenTelemetry configuration

The change from a string-based endpoint (OtelOtlpEndpoint) to a boolean flag (OtelEnabled) simplifies the configuration. This aligns with the PR objective to update the OpenTelemetry integration.

go/pkg/membership/serf.go (1)

15-15:

Details

❓ Verification inconclusive

LGTM: Updated import path for OpenTelemetry integration

The change to use the OpenTelemetry-based logging package aligns with the PR objective to send data to an OpenTelemetry endpoint.

Let's verify that this import change has been made consistently across the codebase:


🏁 Script executed:

#!/bin/bash
# Check for any remaining imports of the old logging package
rg "github.com/unkeyed/unkey/go/pkg/logging" --type go

Length of output: 54


LGTM: Updated OpenTelemetry logging integration across the codebase

  • Verified that the import in go/pkg/membership/serf.go now uses the new path:
    github.com/unkeyed/unkey/go/pkg/otel/logging
  • The search for any remaining references to the old logging package (github.com/unkeyed/unkey/go/pkg/logging) produced no matches.
  • Action: Please manually verify that no edge-case usages of the old logging package remain.
go/apps/api/routes/services.go (1)

9-9: LGTM: Updated import path for OpenTelemetry integration

The change to use the OpenTelemetry-based logging package is consistent with the other files in this PR.

go/pkg/cluster/cluster.go (1)

9-9: LGTM: Updated import path for OpenTelemetry integration

This change aligns with the existing OpenTelemetry metrics integration in this file and the broader changes in the PR.

go/pkg/zen/server.go (1)

11-11: LGTM: Clean import change for OpenTelemetry logging

The change from the standard logging package to OpenTelemetry-based logging looks good. This update aligns with the PR objective of sending telemetry data to an OpenTelemetry endpoint.

go/internal/services/keys/service.go (1)

5-5: LGTM: Consistent logging package update

This import path change maintains consistency with the other files in this PR, properly switching to the OpenTelemetry-based logging package.

go/internal/services/permissions/service.go (1)

5-5: LGTM: Proper migration to OpenTelemetry logging

The import path change is correctly implemented, switching from the standard logging package to the OpenTelemetry implementation while maintaining the same interface.

go/cmd/quotacheck/main.go (2)

15-15: LGTM: Consistent import change for OpenTelemetry

The import statement is correctly updated to use the OpenTelemetry logging package.


59-59:

Details

✅ Verification successful

Verify logger configuration defaults

The logger initialization has been simplified from explicit configuration to using default settings. Ensure that the default behavior of the new OpenTelemetry logger meets the requirements previously set with Development: true, NoColor: false.


🏁 Script executed:

#!/bin/bash
# Find how the new logging.New() function is implemented
find go/pkg/otel -type f -name "*.go" | xargs grep -l "func New" | xargs cat

Length of output: 6175


Logger Defaults Verified – No Further Changes Needed

After reviewing the implementation, the default logger configuration is set in the init() function of the logging package. Specifically, the handler is initialized using:

handler = tint.NewHandler(os.Stdout, &tint.Options{
	AddSource:   false,
	Level:       slog.LevelDebug,
	ReplaceAttr: nil,
	TimeFormat:  time.StampMilli,
	NoColor:     false,
})

This confirms that:

  • NoColor is set to false, ensuring that colors are enabled (consistent with a development mode setting).
  • The log level is set to slog.LevelDebug, aligning with the expected development behavior.
  • The overall configuration supports a human-readable format suitable for development.

The logging.New() function simply wraps this pre-configured handler by calling slog.New(handler), thereby inheriting these default settings. This meets the previous requirements (Development: true, NoColor: false) without needing any explicit configuration on each logger instantiation.

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

27-27: Import path updated for OpenTelemetry logging

The import has been updated to use the new OpenTelemetry-specific logging package, which is consistent with the PR's objective to improve OpenTelemetry integration.


46-58: Simplified OpenTelemetry configuration

The configuration has been improved by:

  1. Using a boolean flag OtelEnabled instead of a string-based endpoint configuration
  2. Providing explicit configuration parameters to InitGrafana including application name, version, node ID, and cloud region

This change makes the telemetry configuration more straightforward and consistent.


60-60: Simplified logger initialization

The logger initialization has been simplified by removing explicit configuration parameters and relying on defaults defined within the package. This is consistent with the approach taken in other files in this PR.

go/pkg/sim/simulation.go (2)

10-10: Import path updated for OpenTelemetry logging

The import has been updated to use the new OpenTelemetry-specific logging package, which is consistent with the PR's objective to improve OpenTelemetry integration.


69-69: Simplified logger initialization

The logger initialization has been simplified by removing the explicit configuration parameters (likely NoColor: false, Development: true) and relying on defaults defined within the package. This makes the code more consistent with other parts of the codebase using the new OpenTelemetry logging.

go/pkg/testutil/http.go (2)

21-21: Import path updated for OpenTelemetry logging

The import has been updated to use the new OpenTelemetry-specific logging package, which is consistent with the PR's objective to improve OpenTelemetry integration.


56-56: Simplified logger initialization

The logger initialization has been simplified by removing the explicit configuration parameters (previously Development: true, NoColor: false) and relying on defaults defined within the package. This makes the code more consistent with other parts of the codebase using the new OpenTelemetry logging.

go/internal/services/ratelimit/sliding_window.go (1)

15-15: Import path updated for OpenTelemetry logging

The import has been updated to use the new OpenTelemetry-specific logging package, which is consistent with the PR's objective to improve OpenTelemetry integration.

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

50-50: No concerns on updated default HTTP port.
The consolidated example improves clarity.


159-159: No concerns on updated default RPC port.
This example remains consistent with the documentation of the default port.


177-177: No concerns on updated default gossip port.
This example is clear and does not raise any issues.


300-321: Well-documented OTEL flag addition.
The introduced boolean flag and usage instructions are clear, improving the user experience for toggling OpenTelemetry.


345-345: Consistent usage of the new OtelEnabled field.
All references to the old endpoint-based configuration appear properly replaced.

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

8-8: Updated imports for OTEL and logging.
These new paths align with the updated OpenTelemetry logging approach.

Also applies to: 12-13, 15-16, 19-19


94-124: Robust OTLP log handler with batch processing.
The addition of logExporter, sdklog.NewBatchProcessor, and minsev.NewLogProcessor successfully leverages OTLP-based logging. Consider monitoring CPU vs. throughput overhead when adjusting buffer sizes.

go/pkg/clickhouse/client.go (3)

12-12: Updated logging import to support OpenTelemetry integration.

This change migrates from the standard logging package to an OpenTelemetry-compatible logging implementation, which is consistent with the PR objective of sending telemetry data to an OpenTelemetry endpoint.


104-104: Table name updated with 'metrics' namespace prefix.

The table name for API requests has been updated to include a namespace prefix, improving database organization by logically separating different types of data.


123-123: Table name updated with 'verifications' namespace prefix.

The table name for key verifications has been updated to include a namespace prefix, improving database organization by logically separating different types of data.

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

111-112: Updated database connection examples with clear use cases.

The updated examples provide better context with one for local development and one for PlanetScale, both with the necessary query parameters.


143-165: Improved OpenTelemetry configuration approach.

This change replaces the specific --otel-otlp-endpoint string parameter with a simpler boolean flag --otel that relies on standard OpenTelemetry environment variables. This is a better approach because:

  1. It aligns with OpenTelemetry conventions
  2. Provides more flexibility in configuration
  3. Makes it easier to document and follow standard practices
  4. Allows for more detailed configuration through environment variables

The updated examples with environment variables are particularly helpful.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~148-~148: Loose punctuation mark.
Context: ...rter: - OTEL_EXPORTER_OTLP_ENDPOINT: The URL of your OpenTelemetry collector...

(UNLIKELY_OPENING_PUNCTUATION)

go/go.mod (2)

27-38: Updated OpenTelemetry dependencies to support logging capabilities.

The dependency updates include:

  1. Adding OpenTelemetry logging bridge and exporters
  2. Updating core OpenTelemetry libraries to newer versions (v1.35.0)
  3. Adding minimum severity processor support

These changes are necessary to support the shift to a more standardized OpenTelemetry approach for telemetry data.


44-44: Updated supporting libraries.

Updates to these supporting libraries (cel.dev/expr, Google Protobuf, and gRPC) are important to maintain compatibility with the updated OpenTelemetry components.

Also applies to: 162-164

@vercel vercel bot temporarily deployed to Preview – play March 14, 2025 10:54 Inactive
@vercel vercel bot temporarily deployed to Preview – engineering March 14, 2025 10:54 Inactive
@vercel vercel bot temporarily deployed to Preview – www March 14, 2025 10:55 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
apps/engineering/content/architecture/services/api/config.mdx (1)

332-332: ⚠️ Potential issue

Update AWS ECS example to use new OpenTelemetry flag

The AWS ECS Production Cluster example still uses the old --otel-otlp-endpoint flag, which has been replaced with the new --otel boolean flag.

-  --otel-otlp-endpoint="https://your-grafana-endpoint.com"
+  --otel=true

You'll also need to add the corresponding environment variables as shown in the new examples:

export OTEL_EXPORTER_OTLP_ENDPOINT="https://your-grafana-endpoint.com"
export OTEL_EXPORTER_OTLP_PROTOCOL="http/protobuf"
export OTEL_EXPORTER_OTLP_HEADERS="authorization=Basic ..."
♻️ Duplicate comments (1)
apps/engineering/content/architecture/services/api/config.mdx (1)

123-124: ⚠️ Potential issue

Incorrect example duplication for read-only replica.

The examples for the read-only replica database are identical to those for the primary database, which appears to be a copy-paste error. The read-only replica examples should demonstrate a replica configuration.

-  - `--database-readonly-replica=mysql://root:password@localhost:3306/unkey?parseTime=true` - Local MySQL for development
-  - `--database-readonly-replica=mysql://username:pscale_pw_...@aws.connect.psdb.cloud/unkey?sslmode=require` - PlanetScale connection
+  - `--database-readonly-replica=mysql://root:password@localhost:3307/unkey?parseTime=true` - Local MySQL read replica for development
+  - `--database-readonly-replica=mysql://username:pscale_pw_...@aws-ro.connect.psdb.cloud/unkey?sslmode=require` - PlanetScale read replica connection
🧹 Nitpick comments (1)
apps/engineering/content/architecture/services/api/config.mdx (1)

148-151: Fix formatting in OpenTelemetry environment variables section

There appears to be some inconsistent formatting with the list of environment variables. Ensure consistent punctuation and indentation.

  When this flag is set to true, the following standard OpenTelemetry environment variables are used to configure the exporter:

-  - `OTEL_EXPORTER_OTLP_ENDPOINT`: The URL of your OpenTelemetry collector
-  - `OTEL_EXPORTER_OTLP_PROTOCOL`: The protocol to use (http/protobuf or grpc)
-  - `OTEL_EXPORTER_OTLP_HEADERS`: Headers for authentication (e.g., "authorization=Bearer \<token\>")
+  - `OTEL_EXPORTER_OTLP_ENDPOINT`: The URL of your OpenTelemetry collector
+  - `OTEL_EXPORTER_OTLP_PROTOCOL`: The protocol to use (http/protobuf or grpc)
+  - `OTEL_EXPORTER_OTLP_HEADERS`: Headers for authentication (e.g., "authorization=Bearer \<token\>")
🧰 Tools
🪛 LanguageTool

[uncategorized] ~148-~148: Loose punctuation mark.
Context: ...rter: - OTEL_EXPORTER_OTLP_ENDPOINT: The URL of your OpenTelemetry collector...

(UNLIKELY_OPENING_PUNCTUATION)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 17726e5 and 62932b0.

📒 Files selected for processing (5)
  • apps/agent/pkg/gossip/membership_test.go (0 hunks)
  • apps/engineering/content/architecture/services/api/config.mdx (3 hunks)
  • go/apps/api/cancel_test.go (1 hunks)
  • go/apps/api/run_test.go (1 hunks)
  • go/cmd/api/main.go (6 hunks)
💤 Files with no reviewable changes (1)
  • apps/agent/pkg/gossip/membership_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • go/cmd/api/main.go
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/architecture/services/api/config.mdx

[uncategorized] ~148-~148: Loose punctuation mark.
Context: ...rter: - OTEL_EXPORTER_OTLP_ENDPOINT: The URL of your OpenTelemetry collector...

(UNLIKELY_OPENING_PUNCTUATION)

⏰ Context from checks skipped due to timeout of 90000ms (17)
  • GitHub Check: Test Packages / Test ./packages/rbac
  • GitHub Check: Test Packages / Test ./packages/hono
  • GitHub Check: Test Packages / Test ./packages/nextjs
  • GitHub Check: Test Packages / Test ./internal/clickhouse
  • GitHub Check: Test Packages / Test ./internal/encryption
  • GitHub Check: Test Packages / Test ./packages/cache
  • GitHub Check: Test Packages / Test ./packages/api
  • GitHub Check: Build / Build
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Packages / Test ./internal/id
  • GitHub Check: Test Packages / Test ./internal/keys
  • GitHub Check: Test Packages / Test ./internal/billing
  • GitHub Check: Test Packages / Test ./internal/hash
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
go/apps/api/cancel_test.go (1)

44-44: Consistent change to OpenTelemetry configuration

The configuration has been updated to use a boolean flag OtelEnabled instead of a string-based endpoint configuration, which aligns with the broader OpenTelemetry refactoring in the codebase. This approach makes the configuration clearer and easier to manage.

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

55-55: Consistent change to OpenTelemetry configuration

The configuration has been updated to use a boolean flag OtelEnabled instead of a string-based endpoint configuration, matching the changes in other files. This creates a more consistent approach to enabling/disabling OpenTelemetry across the codebase.

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

143-166: Well-documented OpenTelemetry configuration update

The documentation has been updated to reflect the change from a string-based endpoint configuration to a boolean flag. The explanation of how to use standard OpenTelemetry environment variables is clear and comprehensive, which will help users understand how to properly configure the system.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~148-~148: Loose punctuation mark.
Context: ...rter: - OTEL_EXPORTER_OTLP_ENDPOINT: The URL of your OpenTelemetry collector...

(UNLIKELY_OPENING_PUNCTUATION)

@chronark chronark merged commit f7a5f30 into main Mar 14, 2025
29 of 30 checks passed
@chronark chronark deleted the otel-do-what-ian-did-and-hope-it-works branch March 14, 2025 11:19
@coderabbitai coderabbitai bot mentioned this pull request Apr 14, 2025
18 tasks
@coderabbitai coderabbitai bot mentioned this pull request Jul 21, 2025
18 tasks
@coderabbitai coderabbitai bot mentioned this pull request Feb 6, 2026
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