[TT-7519] add "original_path" and "listen_path" to observability by sedkis · Pull Request #943 · TykTechnologies/tyk-pump

sedkis · 2026-02-18T20:16:24Z

Enables the gateway to store the original client request path (before
listen path stripping and URL rewriting) alongside the existing backend
path in analytics records.

Step-ID: pump-us002

Co-Authored-By: Claude noreply@anthropic.com

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

Ticket Details

TT-7519


Status	In Code Review
Summary	[Innersource] Include original request path in analytics

Generated at: 2026-02-18 21:25:35

…ecord Enables the gateway to store the original client request path (before listen path stripping and URL rewriting) alongside the existing backend path in analytics records. Step-ID: pump-us002 Co-Authored-By: Claude <noreply@anthropic.com>

probelabs · 2026-02-18T20:20:02Z

This PR enhances observability by adding original_path and listen_path to the analytics record. This allows the gateway to store the original client request path before any modifications like listen path stripping or URL rewriting, providing valuable context for traffic analysis and debugging.

Files Changed Analysis

analytics/analytics.go: The core AnalyticsRecord struct is extended with OriginalPath and ListenPath fields. The GetFieldNames and GetLineValues methods are updated to include them.
analytics/analytics.proto: The protobuf schema for AnalyticsRecord is updated to include the new OriginalPath and ListenPath string fields.
analytics/proto/analytics.pb.go: The auto-generated Go code for protobuf is updated to reflect the schema changes, including new getter methods.
serializer/protobuf.go: The serialization logic is updated to map the new fields between the Go struct and the protobuf message during encoding and decoding.
analytics/analytics_test.go: Tests are updated to accommodate the new fields. A brittle test that asserted on a fixed number of fields was removed, improving test robustness.
serializer/serializer_test.go: A new round-trip test, TestSerializer_OriginalPathAndListenPathRoundTrip, has been added to ensure both msgpack and protobuf serializers correctly handle the new fields.

Architecture & Impact Assessment

What this PR accomplishes: It enriches analytics data by capturing the original, unmodified client request path (original_path) and the API's configured listen path (listen_path). This is crucial for understanding request routing, especially in complex scenarios involving URL rewriting.
Key technical changes introduced:
- Extension of the AnalyticsRecord data model.
- Update to the protobuf data contract for analytics.
- Addition of a new serialization round-trip test for better test coverage.
Affected system components:
- Analytics Data Model: The AnalyticsRecord struct, which is central to the pump's data flow.
- Data Serialization: Both msgpack and protobuf serialization pathways are updated.
- Downstream Data Pumps: This is an additive, non-breaking change. All pumps that process AnalyticsRecord (e.g., for Elasticsearch, SQL, Splunk) will now have access to the new fields, though they will not utilize them without further modification.

graph TD
    A[Client Request] --> B[Tyk Gateway];
    B --> C{Analytics Record Generation};
    C --|Adds original_path and listen_path|--> D[AnalyticsRecord];
    D --> E[Tyk Pump];
    E --> F[Serialization];
    F --> G[Data Sinks e.g., Elasticsearch, SQL];

Scope Discovery & Context Expansion

This change to the core AnalyticsRecord data structure has a wide but non-breaking impact. While the new fields are now present in the data model, making them useful requires follow-up work in the various data pumps and downstream consumers.

Based on the repository structure, the next logical steps would be to modify the pump implementations to handle the new fields:

pumps/elasticsearch.go: The Elasticsearch index mapping would need to be updated to include and index original_path and listen_path.
pumps/sql.go: The SQL pump would require a database schema migration to add originalpath and listenpath columns to the analytics table.
Other Pumps: Similar changes would be needed for other pumps like Splunk (pumps/splunk.go) or Logz.io (pumps/logzio.go) to ensure the new data is passed to the respective backends.

This PR successfully lays the groundwork for these future enhancements.

Metadata

Review Effort: 2 / 5
Primary Label: feature

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:26:40.192Z | Triggered by: pr_updated | Commit: 438c2b2

💡 TIP: You can chat with Visor using /visor ask <your question>

probelabs · 2026-02-18T20:20:35Z

Security Issues (1)

Severity	Location	Issue
🔴 Critical	`analytics/analytics.go:77`	The new `OriginalPath` field stores the full, unsanitized request path, including the query string. As confirmed in the test `TestSerializer_OriginalPathAndListenPathRoundTrip` (`serializer/serializer_test.go`), query parameters are persisted. Storing raw query strings in analytics systems can lead to sensitive data exposure, as they may contain API keys, session tokens, personally identifiable information (PII), or other credentials. This data is then propagated to various data sinks (e.g., Elasticsearch, SQL databases), increasing the risk of data leakage. 💡 Suggestion Implement a sanitization mechanism to filter, mask, or strip sensitive query parameters from the `OriginalPath` before it is stored. This could be a configurable allow/block list of parameter keys. If query parameters are not essential for the intended analytics, they should be stripped entirely.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (2)

Severity	Location	Issue
🟠 Error	`analytics/proto/analytics.pb.go:1`	The pull request fails the quality gate due to insufficient test coverage on new code (50% vs. required 80%). The coverage gap is in the generated protobuf file, where the `nil` receiver path for the new `GetOriginalPath` and `GetListenPath` getters is not tested. As this is a failing CI check, it must be addressed. 💡 Suggestion Add a unit test to cover the `nil` receiver scenario for the new getter methods to satisfy the coverage requirements. For example: ```go func TestAnalyticsRecord_NilReceiverGetters(t testing.T) { var rec proto.AnalyticsRecord = nil assert.Equal(t, "", rec.GetOriginalPath()) assert.Equal(t, "", rec.GetListenPath()) } ```
🟡 Warning	`analytics/analytics_test.go:175-177`	The test `TestAnalyticsRecord_GetLineValues` uses `assert.Contains` within a loop over the struct fields. This is a weak test as it does not validate the order of fields in the returned slice, nor does it ensure against unexpected extra values. A regression that changes the field order would not be caught. 💡 Suggestion Refactor the test to build an expected slice of strings in the correct order and use `assert.Equal` to compare it against the result from `GetLineValues`. This will create a more precise and robust test that properly validates the function's output.

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:26:43.429Z | Triggered by: pr_updated | Commit: 438c2b2

💡 TIP: You can chat with Visor using /visor ask <your question>

sedkis · 2026-02-18T20:58:11Z

/review The OriginalPath field stores the full, unsanitized request path, including the query string. Query strings often contain sensitive information such as API keys, session tokens, password reset tokens, or personally identifiable information (PII). Storing this data in analytics systems can lead to a critical sensitive data leak, exposing it to systems and personnel who should not have access. The test case in serializer/serializer_test.go confirms that query parameters are stored.

We follow existing structure for "rawPath" and "path" today. These can be sanitized down-chain

…each field

probelabs · 2026-02-18T21:00:31Z

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:06:10.905Z | Triggered by: sedkis | Commit: 7c66772

💡 TIP: You can chat with Visor using /visor ask <your question>

probelabs · 2026-02-18T21:00:31Z

Thank you for raising this important security concern. You are correct that storing the OriginalPath with unsanitized query parameters could expose sensitive data in analytics.

This pull request follows an established pattern in tyk-pump. The AnalyticsRecord struct already includes fields like RawPath and RawRequest which also store raw, unsanitized request data.

While this PR is consistent with the existing design, your point about potential data leaks is valid and highlights a broader security consideration. A more comprehensive solution would be to implement a configurable sanitization mechanism to scrub sensitive parameters from all relevant fields, including OriginalPath, RawPath, and RawRequest.

To address this system-wide, I recommend creating a new issue to discuss and track the implementation of a flexible data sanitization feature for analytics. This would allow the concern to be addressed holistically without blocking this particular PR.

References:

analytics/analytics.go:45-80 – AnalyticsRecord struct definition showing RawPath and RawRequest fields.
serializer/serializer_test.go:126-129 – Test case confirming OriginalPath includes query parameters.

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:00:33.510Z | Triggered by: sedkis | Commit: 7c66772

💡 TIP: You can chat with Visor using /visor ask <your question>

sedkis · 2026-02-18T21:02:20Z

/review ive addressed the flaky length test

sedkis · 2026-02-18T21:02:44Z

/review The new fields OriginalPath and ListenPath are mapped to database columns. If these columns are used in WHERE clauses for filtering or sorting on the analytics table, which can grow very large, queries could be very slow without database indexes. Adding indexes would improve query performance but increase storage and write overhead.

they're not used in WHERE clauses

probelabs · 2026-02-18T21:03:01Z

Thanks for addressing the flaky test by removing the hardcoded length assertions. I've re-triggered the checks.

References:

analytics/analytics_test.go – Removed hardcoded length assertions.

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:07:40.435Z | Triggered by: sedkis | Commit: 7c66772

💡 TIP: You can chat with Visor using /visor ask <your question>

probelabs · 2026-02-18T21:04:51Z

You are correct. The new fields OriginalPath and ListenPath are added to the analytics data model but are not used for filtering in any of the data pump implementations in this pull request.

The changes are confined to the data structure and serialization, with no modifications to the database query logic in the pumps/ directory.

References:

Files Changed Summary – Note that no files in pumps/ were modified.

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:04:53.404Z | Triggered by: sedkis | Commit: 7c66772

💡 TIP: You can chat with Visor using /visor ask <your question>

sedkis · 2026-02-18T21:05:15Z

/review i will follow up with the santiziation security issue in another task, you can approve this flow

probelabs · 2026-02-18T21:06:00Z

Acknowledged. The plan to address the sanitization of the OriginalPath field in a separate task is noted. This PR can proceed.

References:

serializer/serializer_test.go:129 – Test case confirming OriginalPath includes query parameters.
analytics/analytics.go:77 – Definition of the OriginalPath field.

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:06:02.287Z | Triggered by: sedkis | Commit: 7c66772

💡 TIP: You can chat with Visor using /visor ask <your question>

sedkis · 2026-02-18T21:15:58Z

/review the visor: quality and visor:security checks are still marked as "Check failed" in CI even though they've been addressed

probelabs · 2026-02-18T21:17:09Z

I've re-triggered the checks as requested.

The visor:security and visor:quality checks are still failing due to the same two underlying issues flagged in the previous review:

Security Finding: The OriginalPath field stores the full, unsanitized request path, which poses a security risk. I acknowledge your comment that this will be addressed in a separate task.
Quality Gate: The SonarQube quality gate is failing due to low test coverage on new code (50% vs. the required 80%). This is primarily because the new code in the auto-generated protobuf file and the updated GetLineValues method are not fully tested.

To address the coverage issue, you could implement the suggestions from the earlier review:

Add a unit test that covers the nil receiver case for the new GetOriginalPath and GetListenPath methods.
Update the TestAnalyticsRecord_GetLineValues test to initialize the OriginalPath and ListenPath fields on the test record.

References:

analytics/proto/analytics.pb.go:375-384 – Generated getters requiring nil-check test coverage.
analytics/analytics_test.go:168-183 – Test case needing updated record initialization.

Powered by Visor from Probelabs

Last updated: 2026-02-18T21:18:59.015Z | Triggered by: sedkis | Commit: 7c66772

💡 TIP: You can chat with Visor using /visor ask <your question>

sonarqubecloud · 2026-02-18T21:30:13Z

Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

sedkis · 2026-02-19T17:40:29Z

@tbuchaillot - the sonarcloud code analysis is failing because (50% code coverage is too low). however -- that is because it is analyzing a PROTO auto-genrated file.

Also, the visor quality + security have been addressed as well.

sedkis changed the title ~~feat(analytics): add OriginalPath and ListenPath fields to AnalyticsRecord~~ [TT-7519] add "original_path" and "listen_path" to observability Feb 18, 2026

Merge remote-tracking branch 'origin' into TT-7519

0e0fb7d

Don't need to assert length, we're already asserting the presence of …

6a7440f

…each field

Fix the goimports

e542f36

add a test case

438c2b2

sedkis requested a review from tbuchaillot February 19, 2026 17:40

Conversation

sedkis commented Feb 18, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

Checklist

Ticket Details

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files Changed Analysis

Architecture & Impact Assessment

Scope Discovery & Context Expansion

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Issues (1)

✅ Architecture Check Passed

✅ Performance Check Passed

Quality Issues (2)

Uh oh!

sedkis commented Feb 18, 2026

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sedkis commented Feb 18, 2026

Uh oh!

sedkis commented Feb 18, 2026

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sedkis commented Feb 18, 2026

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sedkis commented Feb 18, 2026

Uh oh!

probelabs bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 18, 2026

Quality Gate failed

Uh oh!

sedkis commented Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sedkis commented Feb 18, 2026 •

edited by github-actions bot

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading

probelabs bot commented Feb 18, 2026 •

edited

Loading