feat: rename aws tracing module#118
Merged
chalindukodikara merged 1 commit intoMay 18, 2026
Merged
Conversation
Signed-off-by: Chalindu Kodikara <chalindumkodikara@gmail.com>
📝 WalkthroughWalkthroughThe PR rebrands the observability-tracing module from CloudWatch to AWS X-Ray. It systematically updates the Go module path, Helm chart metadata, Kubernetes resource templates, application code imports, and documentation to consistently reference AWS X-Ray instead of CloudWatch across all configuration and deployment artifacts. ChangesAWS X-Ray Observability Module Rebranding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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 (2)
observability-tracing-aws-xray/helm/values.yaml (1)
21-21: 💤 Low valueConsider DRY principle: Secret name duplicated.
The secret name
tracing-aws-xray-aws-credentialsappears in bothawsCredentials.name(line 21) andopentelemetry-collector.extraEnvsFrom[1].secretRef.name(line 58). If a user customizesawsCredentials.name, they must remember to also update theextraEnvsFromreference, creating a maintenance burden.Since Helm values files cannot use templating, this duplication is unavoidable in the values file itself. However, consider addressing this in the templates (e.g., _helpers.tpl could provide a helper function that returns the credentials secret name) or document this coupling clearly in comments.
Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@observability-tracing-aws-xray/helm/values.yaml` at line 21, The values file duplicates the secret name between awsCredentials.name and opentelemetry-collector.extraEnvsFrom[1].secretRef.name; update templates to remove this coupling by introducing a helper in _helpers.tpl (e.g., tpl name like tracing.awsCredentialsSecretName) and use that helper wherever the secret is referenced (templates that render awsCredentials and opentelemetry-collector.extraEnvsFrom) so users only change awsCredentials.name in values.yaml and all template references stay consistent; alternatively add a clear commented note in values.yaml linking awsCredentials.name to opentelemetry-collector.extraEnvsFrom[1].secretRef.name if you prefer documentation only.observability-tracing-aws-xray/README.md (1)
463-467: ⚡ Quick winSimplify static credentials install commands.
The install commands explicitly set
awsCredentials.nameandopentelemetry-collector.extraEnvsFromvalues that already match the chart defaults (from values.yaml lines 21, 56, 58). These--setflags are redundant unless users need to customize the secret name.Consider simplifying the documented commands to omit these redundant flags, or add a note explaining they're shown for explicitness but can be omitted. For example, the single-cluster static install could be reduced to:
helm upgrade --install observability-tracing-aws-xray \ oci://ghcr.io/openchoreo/helm-charts/observability-tracing-aws-xray \ --create-namespace \ --namespace "$NS" \ --version 0.2.0 \ --set region="$AWS_REGION" \ --set awsCredentials.create=true \ --set awsCredentials.accessKeyId="$AWS_ACCESS_KEY_ID" \ --set awsCredentials.secretAccessKey="$AWS_SECRET_ACCESS_KEY"The
awsCredentials.nameandextraEnvsFromwill use their defaults automatically.Also applies to: 503-504
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@observability-tracing-aws-xray/README.md` around lines 463 - 467, The documented helm install command passes redundant --set flags for awsCredentials.name and opentelemetry-collector.extraEnvsFrom which match the chart defaults; remove those redundant flags from the example (leave --set region, --set awsCredentials.create=true, --set awsCredentials.accessKeyId and --set awsCredentials.secretAccessKey) or add a short note that awsCredentials.name and opentelemetry-collector.extraEnvsFrom are shown only for explicitness and can be omitted because they default to the values.yaml settings; update the same simplification for the other occurrence mentioned (lines 503-504 equivalent).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@observability-tracing-aws-xray/helm/templates/_helpers.tpl`:
- Around line 10-12: The validate template "tracing-aws-xray.validate" uses
include truthiness to check region which can miss empty/invalid values because
include returns a string; replace that check with Helm's required function to
enforce a mandatory region (e.g. call required with a descriptive message and
the value obtained from include "tracing-aws-xray.region" or directly from
.Values.region) so the template fails reliably when region is not provided.
In `@observability-tracing-aws-xray/README.md`:
- Line 573: The README table entry for opentelemetry-collector.extraEnvsFrom is
incorrect: update its default value to match values.yaml by including both the
configMapRef and the secretRef (configMapRef: name
tracing-aws-xray-collector-env and secretRef: name
tracing-aws-xray-aws-credentials, optional: true) so the documentation shows the
actual defaults used by the chart.
---
Nitpick comments:
In `@observability-tracing-aws-xray/helm/values.yaml`:
- Line 21: The values file duplicates the secret name between
awsCredentials.name and opentelemetry-collector.extraEnvsFrom[1].secretRef.name;
update templates to remove this coupling by introducing a helper in _helpers.tpl
(e.g., tpl name like tracing.awsCredentialsSecretName) and use that helper
wherever the secret is referenced (templates that render awsCredentials and
opentelemetry-collector.extraEnvsFrom) so users only change awsCredentials.name
in values.yaml and all template references stay consistent; alternatively add a
clear commented note in values.yaml linking awsCredentials.name to
opentelemetry-collector.extraEnvsFrom[1].secretRef.name if you prefer
documentation only.
In `@observability-tracing-aws-xray/README.md`:
- Around line 463-467: The documented helm install command passes redundant
--set flags for awsCredentials.name and opentelemetry-collector.extraEnvsFrom
which match the chart defaults; remove those redundant flags from the example
(leave --set region, --set awsCredentials.create=true, --set
awsCredentials.accessKeyId and --set awsCredentials.secretAccessKey) or add a
short note that awsCredentials.name and opentelemetry-collector.extraEnvsFrom
are shown only for explicitness and can be omitted because they default to the
values.yaml settings; update the same simplification for the other occurrence
mentioned (lines 503-504 equivalent).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e30607f4-25a5-4d4b-98a6-c93a32304246
⛔ Files ignored due to path filters (4)
observability-tracing-aws-xray/go.sumis excluded by!**/*.sumobservability-tracing-aws-xray/helm/Chart.lockis excluded by!**/*.lockobservability-tracing-aws-xray/internal/api/gen/models.gen.gois excluded by!**/gen/**observability-tracing-aws-xray/internal/api/gen/server.gen.gois excluded by!**/gen/**
📒 Files selected for processing (29)
observability-tracing-aws-xray/.dockerignoreobservability-tracing-aws-xray/Dockerfileobservability-tracing-aws-xray/Makefileobservability-tracing-aws-xray/README.mdobservability-tracing-aws-xray/go.modobservability-tracing-aws-xray/helm/.helmignoreobservability-tracing-aws-xray/helm/Chart.yamlobservability-tracing-aws-xray/helm/templates/_helpers.tplobservability-tracing-aws-xray/helm/templates/adapter/deployment.yamlobservability-tracing-aws-xray/helm/templates/adapter/service.yamlobservability-tracing-aws-xray/helm/templates/adapter/serviceaccount.yamlobservability-tracing-aws-xray/helm/templates/aws-credentials/secret.yamlobservability-tracing-aws-xray/helm/templates/collector-env/configmap.yamlobservability-tracing-aws-xray/helm/templates/opentelemetry-collector/configMap.yamlobservability-tracing-aws-xray/helm/values.yamlobservability-tracing-aws-xray/internal/api/cfg-models.yamlobservability-tracing-aws-xray/internal/api/cfg-server.yamlobservability-tracing-aws-xray/internal/config.goobservability-tracing-aws-xray/internal/config_test.goobservability-tracing-aws-xray/internal/handlers.goobservability-tracing-aws-xray/internal/handlers_test.goobservability-tracing-aws-xray/internal/server.goobservability-tracing-aws-xray/internal/xray/client.goobservability-tracing-aws-xray/internal/xray/client_test.goobservability-tracing-aws-xray/internal/xray/queries.goobservability-tracing-aws-xray/internal/xray/queries_test.goobservability-tracing-aws-xray/main.goobservability-tracing-aws-xray/module.yamlobservability-tracing-cloudwatch/helm/templates/collector-env/configmap.yaml
💤 Files with no reviewable changes (1)
- observability-tracing-cloudwatch/helm/templates/collector-env/configmap.yaml
Comment on lines
+10
to
12
| {{- define "tracing-aws-xray.validate" -}} | ||
| {{- if not (include "tracing-aws-xray.region" .) -}} | ||
| {{- fail "region is required. Example: --set region=us-east-1" -}} |
There was a problem hiding this comment.
Use required for region validation instead of include truthiness.
Line 11 can miss invalid input because include returns a string; missing values may not evaluate as empty here, so the fail path can be skipped.
Proposed fix
{{- define "tracing-aws-xray.region" -}}
-{{- .Values.region -}}
+{{- required "region is required. Example: --set region=us-east-1" .Values.region -}}
{{- end -}}
{{- define "tracing-aws-xray.validate" -}}
-{{- if not (include "tracing-aws-xray.region" .) -}}
-{{- fail "region is required. Example: --set region=us-east-1" -}}
-{{- end -}}
+{{- include "tracing-aws-xray.region" . -}}
{{- end -}}In Helm templates, what does `include` return when a referenced value like `.Values.region` is unset, and is `required` the recommended way to enforce mandatory values?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@observability-tracing-aws-xray/helm/templates/_helpers.tpl` around lines 10 -
12, The validate template "tracing-aws-xray.validate" uses include truthiness to
check region which can miss empty/invalid values because include returns a
string; replace that check with Helm's required function to enforce a mandatory
region (e.g. call required with a descriptive message and the value obtained
from include "tracing-aws-xray.region" or directly from .Values.region) so the
template fails reliably when region is not provided.
| | `opentelemetry-collector.image.tag` | `0.151.0` | Collector image tag. | | ||
| | `opentelemetry-collector.serviceAccount.annotations` | `{}` | ServiceAccount annotations for IRSA or other identity integrations. | | ||
| | `opentelemetry-collector.extraEnvsFrom` | `[{configMapRef: {name: tracing-cloudwatch-collector-env}}]` | Extra `envFrom` entries for the collector. The default ConfigMap supplies `AWS_REGION`. Append the static AWS credentials Secret at index `1` on non-EKS clusters. | | ||
| | `opentelemetry-collector.extraEnvsFrom` | `[{configMapRef: {name: tracing-aws-xray-collector-env}}]` | Extra `envFrom` entries for the collector. The default ConfigMap supplies `AWS_REGION`. Append the static AWS credentials Secret at index `1` on non-EKS clusters. | |
There was a problem hiding this comment.
Correct the extraEnvsFrom default value in configuration reference.
The configuration reference states the default is [{configMapRef: {name: tracing-aws-xray-collector-env}}], but the actual default in values.yaml (lines 54-59) includes both a configMapRef AND a secretRef entry:
extraEnvsFrom:
- configMapRef:
name: tracing-aws-xray-collector-env
- secretRef:
name: tracing-aws-xray-aws-credentials
optional: trueUpdate the table entry to reflect the complete default value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@observability-tracing-aws-xray/README.md` at line 573, The README table entry
for opentelemetry-collector.extraEnvsFrom is incorrect: update its default value
to match values.yaml by including both the configMapRef and the secretRef
(configMapRef: name tracing-aws-xray-collector-env and secretRef: name
tracing-aws-xray-aws-credentials, optional: true) so the documentation shows the
actual defaults used by the chart.
yashodgayashan
approved these changes
May 18, 2026
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$subject
Approach
Related Issues
openchoreo/openchoreo#3300
Checklist
Remarks
Summary by CodeRabbit
Documentation
Configuration Updates