-
Notifications
You must be signed in to change notification settings - Fork 3
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: Set 20% otel sample rate for all apps in yt01 #1737
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new parameter Changes
Possibly related PRs
Suggested reviewers
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.azure/applications/web-api-so/staging.bicepparam (1)
9-10
: LGTM! Consider documenting sampling strategy.The 100% sampling rate is consistent with other staging environments. Consider documenting the sampling strategy in a central location to help maintain consistency across services and environments.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.azure/applications/graphql/main.bicep
(2 hunks).azure/applications/graphql/prod.bicepparam
(1 hunks).azure/applications/graphql/staging.bicepparam
(1 hunks).azure/applications/graphql/test.bicepparam
(1 hunks).azure/applications/graphql/yt01.bicepparam
(1 hunks).azure/applications/web-api-eu/main.bicep
(2 hunks).azure/applications/web-api-eu/prod.bicepparam
(1 hunks).azure/applications/web-api-eu/staging.bicepparam
(1 hunks).azure/applications/web-api-eu/test.bicepparam
(1 hunks).azure/applications/web-api-eu/yt01.bicepparam
(1 hunks).azure/applications/web-api-so/main.bicep
(2 hunks).azure/applications/web-api-so/prod.bicepparam
(1 hunks).azure/applications/web-api-so/staging.bicepparam
(1 hunks).azure/applications/web-api-so/test.bicepparam
(1 hunks).azure/applications/web-api-so/yt01.bicepparam
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Dry run deploy apps / Deploy job sync-resource-policy-information-job to test
- GitHub Check: Dry run deploy apps / Deploy job sync-subject-resource-mappings-job to test
- GitHub Check: build / build-and-test
🔇 Additional comments (15)
.azure/applications/graphql/test.bicepparam (1)
9-9
: Verify if 100% sampling rate is intended for test environment.The PR title suggests setting 20% sampling rate for all apps, but this file sets
otelTraceSamplerRatio = '1'
(100% sampling). Please confirm if this is intentional or if it should be set to '0.2' (20%) as mentioned in the PR title..azure/applications/web-api-eu/prod.bicepparam (1)
9-9
: Verify if 100% sampling rate is intended for prod environment.The PR title suggests setting 20% sampling rate for all apps, but this file sets
otelTraceSamplerRatio = '1'
(100% sampling). Please confirm if this is intentional for the production environment..azure/applications/graphql/prod.bicepparam (1)
9-9
: Verify if 100% sampling rate is intended for prod environment.The PR title suggests setting 20% sampling rate for all apps, but this file sets
otelTraceSamplerRatio = '1'
(100% sampling). Please confirm if this is intentional for the production environment..azure/applications/web-api-so/test.bicepparam (1)
9-9
: Verify if 100% sampling rate is intended for test environment.The PR title suggests setting 20% sampling rate for all apps, but this file sets
otelTraceSamplerRatio = '1'
(100% sampling). Please confirm if this is intentional or if it should be set to '0.2' (20%) as mentioned in the PR title..azure/applications/web-api-eu/test.bicepparam (1)
9-10
: Verify if 100% sampling rate is intended for test environment.The PR title suggests setting 20% sampling rate for all apps, but this configuration sets it to 100% (
'1'
). While full sampling can be beneficial in test environments for debugging, it may impact performance and cost. Please confirm if this is intentional..azure/applications/graphql/yt01.bicepparam (1)
9-10
: LGTM! Sampling rate aligns with PR objective.The 20% sampling rate (
'0.2'
) matches the PR's goal and is appropriate for the YT01 environment..azure/applications/web-api-so/yt01.bicepparam (1)
9-10
: LGTM! Sampling rate aligns with PR objective.The 20% sampling rate (
'0.2'
) is:
- Consistent with the PR's goal
- Matches the sampling rate in other YT01 environments
- Provides a good balance between telemetry insights and performance
However, there seems to be an inconsistency across environments:
- YT01 environments use 20% sampling
- Other environments (test, prod) use 100% sampling
This mixed configuration doesn't align with the PR title's goal of "Set 20% otel sample rate for all apps". Please clarify if this is intentional or if all environments should use 20% sampling.
.azure/applications/web-api-eu/yt01.bicepparam (1)
9-10
: LGTM! Verify OpenTelemetry configuration.The 20% sampling rate is appropriate for the yt01 environment to reduce telemetry volume while maintaining useful traces for debugging.
Let's verify the OpenTelemetry configuration across all environments:
✅ Verification successful
OpenTelemetry sampling configuration is correctly implemented across environments
The sampling configuration follows best practices with:
- 20% sampling in yt01 (development) to reduce telemetry volume
- 100% sampling in test, staging, and production for full observability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check OpenTelemetry sampling rates across environments echo "Checking OpenTelemetry sampling rates across environments..." fd -e bicepparam -x grep -l "otelTraceSamplerRatio" {} \; | while read -r file; do echo -e "\n$file:" grep "otelTraceSamplerRatio" "$file" doneLength of output: 206
Script:
#!/bin/bash # First, find all bicepparam files echo "Finding all .bicepparam files:" fd -e bicepparam echo -e "\nChecking contents of bicepparam files for any OpenTelemetry config:" fd -e bicepparam -x cat {}Length of output: 271
Script:
#!/bin/bash # Check if the .azure/applications directory exists and list its contents echo "Checking .azure/applications directory:" ls -la .azure/applications 2>/dev/null || echo "Directory not found" echo -e "\nChecking full repository structure:" ls -laLength of output: 3036
Script:
#!/bin/bash echo "Checking web-api-eu directory contents:" ls -la .azure/applications/web-api-eu/ echo -e "\nChecking OpenTelemetry docker compose configuration:" cat docker-compose-otel.yml echo -e "\nChecking local OpenTelemetry configuration:" ls -la local-otel-configuration/Length of output: 3511
Script:
#!/bin/bash echo "Comparing OpenTelemetry configuration across environments:" for env in yt01 test staging prod; do echo -e "\n=== $env environment ===" cat ".azure/applications/web-api-eu/${env}.bicepparam" doneLength of output: 3364
.azure/applications/graphql/staging.bicepparam (1)
9-10
: LGTM! Full sampling appropriate for staging.The 100% sampling rate in staging environment allows comprehensive monitoring and debugging capabilities.
.azure/applications/web-api-eu/staging.bicepparam (1)
9-10
: LGTM! Verify main.bicep implementation.The 100% sampling rate is consistent with other staging environments.
Let's verify the environment variable configuration in the main.bicep files:
✅ Verification successful
Implementation verified: otelTraceSamplerRatio is correctly configured
The parameter is properly integrated in main.bicep:
- Declared with appropriate description and validation
- Correctly passed to OTEL_TRACES_SAMPLER_ARG environment variable
- Used in conjunction with parentbased_traceidratio sampler type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check how the sampling ratio is used in main.bicep files echo "Checking main.bicep implementations..." fd main.bicep -x grep -A 5 -B 5 "OTEL_TRACES_SAMPLER" {} \;Length of output: 149
Script:
#!/bin/bash # Search for otelTraceSamplerRatio parameter and related configurations echo "Searching for otelTraceSamplerRatio in main.bicep..." fd main.bicep -x grep -A 5 -B 5 "otelTraceSamplerRatio" {} \; echo -e "\nSearching for OTEL related configurations..." fd main.bicep -x grep -A 5 -B 5 "OTEL" {} \;Length of output: 332
Script:
#!/bin/bash # Check the specific main.bicep file content echo "Checking web-api-eu main.bicep content..." cat .azure/applications/web-api-eu/main.bicep echo -e "\nChecking all param declarations in main.bicep..." fd main.bicep -x grep -A 1 -B 1 "param " {} \;Length of output: 5025
.azure/applications/graphql/main.bicep (1)
91-98
: LGTM! OpenTelemetry configuration looks good.The environment variables are correctly configured:
OTEL_TRACES_SAMPLER
uses the recommendedparentbased_traceidratio
samplerOTEL_TRACES_SAMPLER_ARG
is properly linked to the sampling ratio parameter.azure/applications/web-api-eu/main.bicep (2)
48-50
: Parameter validation needs improvement.Same validation enhancement needed as in
graphql/main.bicep
.
94-101
: LGTM! OpenTelemetry configuration is consistent.The environment variables match the configuration in
graphql/main.bicep
..azure/applications/web-api-so/main.bicep (2)
48-50
: Parameter validation needs improvement.Same validation enhancement needed as in other Bicep files.
122-129
: LGTM! OpenTelemetry configuration is consistent.The environment variables match the configuration in other Bicep files.
🤖 I have created a release *beep* *boop* --- ## [1.47.3](v1.47.2...v1.47.3) (2025-01-28) ### Bug Fixes * **graphql:** Use correct type filter for LocalDevelopmentUser ([#1745](#1745)) ([14ff138](14ff138)) * Use correct type filter for LocalDevelopmentUser ([#1744](#1744)) ([fa30ebe](fa30ebe)) ### Miscellaneous Chores * **deps:** update dependency npgsql.entityframeworkcore.postgresql to 9.0.3 ([#1734](#1734)) ([195443f](195443f)) * **deps:** update dependency verify.xunit to 28.9.0 ([#1735](#1735)) ([73d1ddb](73d1ddb)) * **deps:** update dependency vitest to v3 ([#1732](#1732)) ([9e67931](9e67931)) * **deps:** update dependency vitest to v3.0.2 ([#1733](#1733)) ([f32a0e2](f32a0e2)) * **deps:** update opentelemetry-dotnet monorepo to 1.11.0 ([#1736](#1736)) ([75c7a24](75c7a24)) * Include chores in the changelog ([#1525](#1525)) ([d9281fc](d9281fc)) * Set 20% otel sample rate for all apps in yt01 ([#1737](#1737)) ([09c9ce9](09c9ce9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
No description provided.