Skip to content

LCORE-1983: Added e2e scenario for OpenTelemetry#1683

Merged
radofuchs merged 1 commit into
lightspeed-core:mainfrom
asimurka:otel_e2e_tests
May 6, 2026
Merged

LCORE-1983: Added e2e scenario for OpenTelemetry#1683
radofuchs merged 1 commit into
lightspeed-core:mainfrom
asimurka:otel_e2e_tests

Conversation

@asimurka
Copy link
Copy Markdown
Contributor

@asimurka asimurka commented May 5, 2026

Description

Adds an e2e test scenario for OpenTelemetry. The scenario tests whether Otel events are exported and successfully received by configured Otel service. The remaining functionality will be tested by integration tests.

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: Cursor

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Tests
    • Added an end-to-end Gherkin test validating OpenTelemetry observability: it exercises an authorized responses request, verifies HTTP 200, and confirms the OpenTelemetry pipeline exports and receives a delivery marker while ensuring the service is started and configured for OTLP. The new feature is included in the E2E test list.

@asimurka asimurka requested a review from radofuchs May 5, 2026 08:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

Warning

Rate limit exceeded

@asimurka has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 47 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: faf35f44-ab58-47eb-be6d-0a18ebcfb37d

📥 Commits

Reviewing files that changed from the base of the PR and between 820055a and 56bd475.

📒 Files selected for processing (2)
  • tests/e2e/features/opentelemetry.feature
  • tests/e2e/test_list.txt

Walkthrough

Adds a new end-to-end Gherkin feature that validates OpenTelemetry export/OTLP ingestion for an authorized responses request using a delivery marker to assert telemetry delivery.

Changes

OpenTelemetry E2E Test Suite

Layer / File(s) Summary
Test Environment Setup
tests/e2e/features/opentelemetry.feature
Background steps start/restart the local service, configure an OTLP/OpenTelemetry listener and exporter, set Bearer Authorization, set REST prefix to /v1, and point to the e2e Lightspeed stack config including lightspeed-stack-auth-noop-token.yaml.
Telemetry Verification Scenario
tests/e2e/features/opentelemetry.feature
Scenario issues an authorized responses request with safety_identifier: "e2e-otel-delivery-marker", expects HTTP 200, and asserts the exported event and OTLP collector both contain the marker.
Test Registration
tests/e2e/test_list.txt
Adds features/opentelemetry.feature to the list of E2E features to execute.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an end-to-end test scenario for OpenTelemetry, which is the primary focus of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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.

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/e2e/features/opentelemetry.feature`:
- Line 9: The feature contains a hardcoded JWT in the step "And I set the
Authorization header to Bearer ..."; remove that literal token and change the
step to use an env/fixture-backed placeholder (e.g., reference a variable like
ACCESS_TOKEN or a test fixture token) so the header is set from process.env or
the test fixture at runtime; update the step implementation that reads the
header (the step definition handling "I set the Authorization header to Bearer")
to pull the token from the environment/fixture when a placeholder is provided.
- Around line 24-25: Replace the fixed "safety_identifier" string with a
per-scenario unique value (e.g., a UUID or timestamp) generated in the scenario
setup (Before/Background) and injected into the payload under
"safety_identifier"; then use that same generated value in both the export
assertion and the collector assertion so each run checks the exact marker it
emitted (reference the "safety_identifier" field and the scenario's
export/collector assertions to locate where to set and compare the value).
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 735a5c8b-672f-43f0-a85e-ac4808de0790

📥 Commits

Reviewing files that changed from the base of the PR and between d0ed69a and ca50ddd.

📒 Files selected for processing (1)
  • tests/e2e/features/opentelemetry.feature
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Pyright
  • GitHub Check: radon
  • GitHub Check: spectral
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{feature,py}

📄 CodeRabbit inference engine (AGENTS.md)

Use behave (BDD) framework for end-to-end testing with Gherkin feature files

Files:

  • tests/e2e/features/opentelemetry.feature

Comment thread tests/e2e/features/opentelemetry.feature
Comment thread tests/e2e/features/opentelemetry.feature
@asimurka asimurka force-pushed the otel_e2e_tests branch 2 times, most recently from b20882d to 820055a Compare May 5, 2026 09:59
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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/e2e/features/opentelemetry.feature`:
- Line 1: The feature is marked with the `@skip` tag which prevents the new
OpenTelemetry e2e scenario from running; edit the feature tag line that
currently contains "@e2e_group_1 `@Authorized` `@skip`" and remove the "@skip" token
so the feature will be executed in CI while leaving the other tags intact.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f0e29dc-83c0-46c5-a59c-370eaa2569da

📥 Commits

Reviewing files that changed from the base of the PR and between ca50ddd and 820055a.

📒 Files selected for processing (2)
  • tests/e2e/features/opentelemetry.feature
  • tests/e2e/test_list.txt
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
  • GitHub Check: mypy
  • GitHub Check: spectral
  • GitHub Check: Pylinter
  • GitHub Check: build-pr
  • GitHub Check: Pyright
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/test_list.txt

📄 CodeRabbit inference engine (AGENTS.md)

End-to-end test list maintained in tests/e2e/test_list.txt

Files:

  • tests/e2e/test_list.txt
tests/e2e/**/*.{feature,py}

📄 CodeRabbit inference engine (AGENTS.md)

Use behave (BDD) framework for end-to-end testing with Gherkin feature files

Files:

  • tests/e2e/features/opentelemetry.feature
🔇 Additional comments (2)
tests/e2e/features/opentelemetry.feature (2)

9-9: Hardcoded bearer token is still committed at Line 9.

Please replace the literal JWT with an env/fixture-backed placeholder so credentials are sourced at runtime, not from versioned test files.


24-25: Static telemetry marker at Line 24 can cause cross-run false positives.

Use a per-scenario unique marker and assert that exact value at Lines 28-29.

Also applies to: 28-29

Comment thread tests/e2e/features/opentelemetry.feature Outdated
Copy link
Copy Markdown
Contributor

@radofuchs radofuchs left a comment

Choose a reason for hiding this comment

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

LGTM

@radofuchs radofuchs merged commit b8e7024 into lightspeed-core:main May 6, 2026
30 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