Minor Changes to the Fix added for Suppressing two Analytics Event per LLM proxy call and adding Integration and Unit Tests

[O-sura]

Purpose

This PR adds some minor improvements to the fix added for suppressing the LLMProvider call analytics event emitted when invoking an LLMProxy. Along with that, this will add both unit tests and integration tests for verifying the intended behavior is provided.

Correctness / clarity

• Replaced the two-case switch in prepareAnalyticEvent with a named helper, isInternalLoopbackHop, using flat guard clauses — the empty-address case no longer sits as a switch branch alongside the "guard says yes" case
• Moved the direct-peer address extraction next to its comment explaining why it's trusted (never derived from X-Forwarded-For), and restored the userIP/userAgent normalisation pair that had been split apart
• Corrected the PropInternalLoopbackProvider doc comment: it claimed the Moesif publisher reads the flag and the traffic log still emits the event, neither of which is true — Process drops the event before the publisher fan-out

Logging / observability

• Throttled the "direct remote address unavailable" warning to once per process via a sync.Once on the Analytics struct — the condition holds for every proxied LLM call when it holds at all, so per-request logging would flood
• Added a per-request debug line for the same condition, so enabling debug reveals whether it affects all traffic or only one ingress path

Tests

• mockPublisher gained a count field; called bool couldn't distinguish one published event from two
• TestProcess_ProxyCallPublishesExactlyOneEvent — feeds both hops of a proxy call and asserts exactly one event survives, and that it's the LlmProxy one (the hop carrying user identity)
• TestProcess_DirectProviderCallPublishesExactlyOneEvent — the counterpart: a direct provider call is one hop and must still count once
• New IT scenario in analytics-basic.feature: deploys a provider + proxy, invokes once, asserts an exact count of 1 — every pre-existing analytics assertion uses at least N, which would pass straight through the original double-counting bug

Related PRs

• Fix issue of emitting two analytics event per LLM proxy call #2879

[coderabbitai]

📝 Walkthrough

Walkthrough

Analytics now suppresses duplicate internal LLM provider loopback events, fails open when direct peer data is missing, and verifies single-event publication through unit and end-to-end tests.

Changes

Analytics deduplication

Layer / File(s)	Summary
Internal loopback suppression gateway/gateway-runtime/policy-engine/internal/analytics/analytics.go, gateway/system-policies/analytics/analytics.go	Detects internal LLM provider loopback hops using metadata and direct peer addresses, with one-time warnings and fail-open behavior when peer data is unavailable.
Publication-count regression tests gateway/gateway-runtime/policy-engine/internal/analytics/analytics_test.go	Counts publisher calls and verifies exactly one event for proxy and direct provider processing.
End-to-end analytics validation gateway/it/features/analytics-basic.feature, gateway/gateway-controller/pkg/eventlistener/llm_provider_processor_test.go	Validates one proxy analytics event and applies a formatting-only test adjustment.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant LlmProxy
  participant LlmProvider
  participant Analytics
  Client->>LlmProxy: Send chat completions request
  LlmProxy->>LlmProvider: Forward request
  LlmProxy->>Analytics: Process proxy hop
  LlmProvider->>Analytics: Process provider hop
  Analytics->>Analytics: Detect internal loopback
  Analytics-->>Client: Publish one proxy analytics event

Loading

Possibly related PRs

• wso2/api-platform#2879: Implements related internal-loopback analytics suppression and associated tests.

Suggested reviewers: malinthaprasan, renuka-fernando, anugayan

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers purpose and tests, but it omits most required template sections like Goals, Approach, Security checks, and Test environment.	Add the missing template sections: Goals, Approach, User stories, Documentation, Automation tests details, Security checks, Samples, and Test environment.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title is specific and accurately reflects the analytics suppression fix plus unit and integration tests.

✨ Finishing Touches

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@gateway/gateway-runtime/policy-engine/internal/analytics/analytics_test.go`:
- Around line 469-503: Extend
TestProcess_DirectProviderCallPublishesExactlyOneEvent with a marked LlmProvider
entry whose direct peer is empty, then assert it still publishes exactly one
event and retains the LlmProvider API type. Use the existing entry-construction
helpers and preserve the current unmarked direct-provider coverage.

In `@gateway/it/features/analytics-basic.feature`:
- Around line 131-136: Replace the fixed 3-second delay after the deployment
response in the analytics scenario with the existing endpoint-readiness step for
/analytics-dedup-proxy/chat/completions, then keep the existing Given I reset
the analytics collector step immediately afterward.

🪄 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: CHILL

Plan: Pro Plus

Run ID: c806387b-b889-4657-96ca-fa8b347ac9bd

📥 Commits

Reviewing files that changed from the base of the PR and between dcfa7e1 and 447d5a7.

📒 Files selected for processing (5)

• gateway/gateway-controller/pkg/eventlistener/llm_provider_processor_test.go
• gateway/gateway-runtime/policy-engine/internal/analytics/analytics.go
• gateway/gateway-runtime/policy-engine/internal/analytics/analytics_test.go
• gateway/it/features/analytics-basic.feature
• gateway/system-policies/analytics/analytics.go