-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Switch server tracing configs to use OTEL #37333
Conversation
WalkthroughThe pull request introduces significant updates to the Appsmith server's dependency management and configuration files. Key changes include the upgrade of OpenTelemetry dependencies, the removal of outdated components, and the addition of new properties for OpenTelemetry tracing in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
🔇 Additional comments (3)app/server/appsmith-server/pom.xml (2)
The switch from Brave to OpenTelemetry tracing looks good, but we should ensure all required dependencies are included. #!/bin/bash
# Check if we have all necessary OTEL dependencies
# Common missing dependencies: opentelemetry-api, opentelemetry-context
rg -g "pom.xml" "opentelemetry-api|opentelemetry-context"
# Check for any existing OTEL configuration that might need updates
rg -g "application*.properties|application*.yml" "opentelemetry|otel"
The major version jump from 1.34.1 to 1.43.0 requires verification of compatibility with other dependencies. ✅ Verification successfulOpenTelemetry 1.43.0 upgrade is compatible with the current setup The version upgrade is compatible with:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for any compatibility issues between OpenTelemetry 1.43.0 and Spring Boot
rg -g "*.xml" -A 5 "opentelemetry|micrometer"
# Check for any migration guides or breaking changes
curl -s https://raw.githubusercontent.com/open-telemetry/opentelemetry-java/main/CHANGELOG.md | grep -A 10 "1.43.0"
Length of output: 278 Script: #!/bin/bash
# Search for OpenTelemetry BOM and related dependencies in pom.xml files
rg -g "pom.xml" -B 2 -A 2 "<version>1\.43\.0</version>"
# Search for Spring Boot version and micrometer dependencies
rg -g "pom.xml" -B 2 -A 2 "spring-boot-starter-parent|micrometer-tracing"
# Look for any OpenTelemetry configuration in properties files
fd -e properties -e yml -x cat {} \; | rg "opentelemetry|otel"
Length of output: 2514 app/server/appsmith-server/src/main/resources/application.properties (1)
The new OpenTelemetry tracing configurations are properly added and should integrate with the tracing system as expected. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Number
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Tests have not run on the HEAD 20454b5 yet
Tue, 12 Nov 2024 07:55:00 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Chores