Skip to content

Update observability and setup to work for oc 0.16#519

Merged
hanzjk merged 5 commits intowso2:mainfrom
AnoshanJ:feat/oauth-sec
Mar 10, 2026
Merged

Update observability and setup to work for oc 0.16#519
hanzjk merged 5 commits intowso2:mainfrom
AnoshanJ:feat/oauth-sec

Conversation

@AnoshanJ
Copy link
Copy Markdown
Contributor

@AnoshanJ AnoshanJ commented Mar 10, 2026

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues in the following format: Resolves issue1, issue2, etc.

Changes to Observability to work with OC 0.16

Goals

Describe the solutions that this feature/fix will introduce to resolve the problems described above

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI (email documentation@wso2.com to review all UI text). Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

User stories

Summary of user stories addressed by this change>

Release note

Brief description of the new feature or bug fix as it will appear in the release notes

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter �N/A� plus brief explanation of why there�s no doc impact

Training

Link to the PR for changes to the training content in https://github.com/wso2/WSO2-Training, if applicable

Certification

Type �Sent� when you have provided new/updated certification questions, plus four answers for each question (correct answer highlighted in bold), based on this change. Certification questions/answers should be sent to certification@wso2.com and NOT pasted in this PR. If there is no impact on certification exams, type �N/A� and explain why.

Marketing

Link to drafts of marketing content that will describe and promote this feature, including product page changes, technical articles, blog posts, videos, etc., if applicable

Automation tests

  • Unit tests

    Code coverage information

  • Integration tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Migrations (if applicable)

Describe migration steps and platforms on which migration has been tested

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Learning

Describe the research phase and any blog posts, patterns, libraries, or add-ons you used to solve the problem.

Summary by CodeRabbit

  • Chores

    • Updated OpenChoreo API endpoint to use port 8195
    • Added port-forwarding support for the OpenChoreo API
    • Made the OpenChoreo setup script more robust and portable (safer configuration replacement)
  • Improvements

    • Queries and tests updated to use keyword-analyzed fields for consistency and reliability
    • Enhanced error logging for better diagnostics during search/aggregation failures

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 10, 2026

📝 Walkthrough

Walkthrough

Updated OpenChoreo service endpoints and port-forwarding, made the OpenChoreo setup script use a cross‑platform sed approach with verification, improved OpenSearch error logging, and migrated OpenSearch queries and tests to use .keyword fields consistently.

Changes

Cohort / File(s) Summary
Env & Compose
agent-manager-service/.env.example, deployments/docker-compose.yml
Updated OPEN_CHOREO_BASE_URL from port 8080 to 8195.
Port forwarding script
deployments/scripts/port-forward.sh
Added kubectl port-forward for OpenChoreo API: prints status and forwards local 8195 to service 8080 in openchoreo-control-plane.
OpenChoreo setup script
deployments/scripts/setup-openchoreo.sh
Replaced single sed -i call with cross-platform detection and in‑place substitution logic (detects sed flavor), sets config_file, source_uri, target_uri, performs substitution, and validates the rewrite; exits on failure.
OpenSearch error logging
traces-observer-service/opensearch/client.go
Enhanced SearchWithCompositeAggregation error handling to read and include the response body (and status) in logs when an error response is returned.
OpenSearch queries & tests
traces-observer-service/opensearch/queries.go, traces-observer-service/opensearch/queries_test.go, traces-observer-service/opensearch/composite_queries_test.go
Migrated field references in queries and tests to use keyword subfields (e.g., traceId.keyword, spanId.keyword, parentSpanId.keyword, component-uid.keyword, environment-uid.keyword, resource.*.uid.keyword) and updated related aggregations/collapse settings and assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I tap the keys and hop with glee,
URLs moved to port eight-one-nine-five, you see,
Scripts now check and fix with care,
Queries ask for keywords everywhere,
A little rabbit cheers: "All set and merry!"

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is largely incomplete, providing only a Purpose statement while leaving all other required template sections unfilled with placeholder text. Complete the remaining sections (Goals, Approach, User stories, Release note, Documentation, Training, Certification, Marketing, Automation tests, Security checks, Samples, Related PRs, Migrations, Test environment, Learning) with specific details about the changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objective of updating observability and setup components to work with OpenChoreo version 0.16.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@AnoshanJ AnoshanJ changed the title Fix issues with observability to work for oc 0.16 Update observability and setup to work for oc 0.16 Mar 10, 2026
@AnoshanJ AnoshanJ requested a review from hanzjk March 10, 2026 10:39
Copy link
Copy Markdown
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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deployments/scripts/port-forward.sh`:
- Around line 70-71: The kubectl port-forward call for the OpenChoreo API
("kubectl port-forward svc/openchoreo-api -n openchoreo-control-plane
8195:8080") is missing a trailing ampersand and runs in the foreground, blocking
subsequent status checks and wait logic; update that invocation to run in the
background (append &), mirroring the other port-forward invocations so the
status block and final wait/trap behavior function correctly.

In `@deployments/scripts/setup-openchoreo.sh`:
- Line 369: The sed invocation that rewrites the JWKS URL (the line that edits
api-platform-operator-local-config.yaml replacing
http://amp-api.wso2-amp.svc.cluster.local:9000/auth/external/jwks.json with
http://host.k3d.internal:9000/auth/external/jwks.json) uses BSD-only `sed -i ''`
and is brittle if the source line changes; make it portable and fail-fast by
using a POSIX-friendly in-place approach (e.g., create a temporary/backup file
with sed -i.bak or use perl -pi) and after the edit verify the replacement
actually occurred (check that the new JWKS URL is present and the old one is
absent), and if verification fails exit with a non-zero status and a clear error
message so the setup script stops rather than silently leaving the wrong JWKS
URI.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e8cafca3-4262-4d14-af95-6b291b7f38fd

📥 Commits

Reviewing files that changed from the base of the PR and between 0e9bb4b and f58106d.

📒 Files selected for processing (8)
  • agent-manager-service/.env.example
  • deployments/docker-compose.yml
  • deployments/scripts/port-forward.sh
  • deployments/scripts/setup-openchoreo.sh
  • traces-observer-service/opensearch/client.go
  • traces-observer-service/opensearch/composite_queries_test.go
  • traces-observer-service/opensearch/queries.go
  • traces-observer-service/opensearch/queries_test.go

Comment thread deployments/scripts/port-forward.sh Outdated
Comment thread deployments/scripts/setup-openchoreo.sh Outdated
Comment thread deployments/docker-compose.yml
Copy link
Copy Markdown
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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deployments/scripts/port-forward.sh`:
- Around line 70-73: The status block in the port-forward script is missing an
entry for the new OpenChoreo Api forward (kubectl port-forward
svc/openchoreo-api mapping 8195:8080 added in
deployments/scripts/port-forward.sh); update the status section that lists
active forwards to include a line for "OpenChoreo Api — localhost:8195 ->
svc/openchoreo-api:8080" (place it alongside the other status entries in the
status block after the other forwards) so users can see the new port-forward in
the script's status output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 38c651d7-6c4b-4201-ac5d-95466fd1d216

📥 Commits

Reviewing files that changed from the base of the PR and between f58106d and a67c99a.

📒 Files selected for processing (2)
  • deployments/scripts/port-forward.sh
  • deployments/scripts/setup-openchoreo.sh

Comment on lines +70 to +73
echo "Forwarding OpenChoreo Api (8195)..."
kubectl port-forward svc/openchoreo-api -n openchoreo-control-plane 8195:8080 &


Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing status entry for the new port-forward.

The OpenChoreo Api port-forward is added but the status block (lines 76-83) doesn't include it. All other port-forwards have their status listed for user reference.

Proposed fix to add the status line

Add the following line in the status block (e.g., after line 83):

 echo "   OpenBao (Secrets):    http://localhost:8200"
+echo "   OpenChoreo Api:       http://localhost:8195"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deployments/scripts/port-forward.sh` around lines 70 - 73, The status block
in the port-forward script is missing an entry for the new OpenChoreo Api
forward (kubectl port-forward svc/openchoreo-api mapping 8195:8080 added in
deployments/scripts/port-forward.sh); update the status section that lists
active forwards to include a line for "OpenChoreo Api — localhost:8195 ->
svc/openchoreo-api:8080" (place it alongside the other status entries in the
status block after the other forwards) so users can see the new port-forward in
the script's status output.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@AnoshanJ, shall we fix this as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Seems already merged. Yes we can add this in another PR then.

@hanzjk hanzjk merged commit 169ad4f into wso2:main Mar 10, 2026
11 checks passed
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