Skip to content

Confirm OTLP observability config for load runs - #2514

Open
Md-Humair-KK wants to merge 1 commit into
mosip:develop-gofrom
Infosys:perf-2497
Open

Confirm OTLP observability config for load runs#2514
Md-Humair-KK wants to merge 1 commit into
mosip:develop-gofrom
Infosys:perf-2497

Conversation

@Md-Humair-KK

@Md-Humair-KK Md-Humair-KK commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Added deployment configuration for enabling or disabling observability through YAML or the MOSIP_ESIGNET_OBSERVABILITY_ENABLED environment variable.
    • Environment settings now override configured values only when valid; otherwise, the configured value is retained.
  • Documentation

    • Added performance-testing guidance explaining the current observability and tracing behavior, audit trail impact, and configuration considerations.

Signed-off-by: mdhumair.kankudti <mdhumair.kankudti@infosys.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Team

Run ID: dae10dd8-4fa8-428a-a4f8-683d0aa71ea6

📥 Commits

Reviewing files that changed from the base of the PR and between 28803cf and e6faf72.

📒 Files selected for processing (4)
  • esignet-service/data/deployment.yaml
  • esignet-service/internal/config/app.go
  • esignet-service/internal/config/app_test.go
  • performance-test/README.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change adds observability configuration, preserves YAML values unless a valid environment override exists, adds tests for resolution behavior, and documents the current tracing implementation status.

Changes

Observability configuration

Layer / File(s) Summary
Configuration resolution and validation
esignet-service/data/deployment.yaml, esignet-service/internal/config/app.go, esignet-service/internal/config/app_test.go, performance-test/README.md
The deployment declares observability.enabled. Application defaults now honor valid MOSIP_ESIGNET_OBSERVABILITY_ENABLED values and retain YAML values for invalid or absent overrides. Tests cover these cases. The performance documentation records that tracing is currently inactive.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e6faf

This change preserves configured observability settings while allowing explicit environment overrides, without activating telemetry or changing externally reachable behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: anushasunkada, sacrana0

Poem

YAML sets the quiet switch
Environment may change its pitch
Tests guard true and false in flight
Invalid values keep things right
Traces sleep beneath the light

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: confirming OTLP observability configuration and its impact on load runs. It is concise and specific.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@codecov-commenter

codecov-commenter commented Sep 2, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop-go@28803cf). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop-go    #2514   +/-   ##
=============================================
  Coverage              ?   70.40%           
=============================================
  Files                 ?      129           
  Lines                 ?     8909           
  Branches              ?      111           
=============================================
  Hits                  ?     6272           
  Misses                ?     2176           
  Partials              ?      461           
Flag Coverage Δ
go 69.27% <100.00%> (?)
npm 92.20% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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