feat: add configuration option to normalize URLs on HTTP events (Fetch and XHR plugins)#650
feat: add configuration option to normalize URLs on HTTP events (Fetch and XHR plugins)#650sergioflores-j wants to merge 1 commit intoaws-observability:mainfrom
Conversation
|
This is a very good PR, and I've seen this pattern used a lot. It also addresses #545 If our purpose is to hide PID, then could keep in extensibility in mind? It might make sense to consolidate the pid scrubbing/transforming logic |
@williazz I haven't considered scrubbing the whole HTTP event as my use case was mostly URL normalization (and not really PII can be sent via the URL in my case), but that sounds like a nice idea to have! Do you have any desired interface in mind? 1. Exposing a callback function (similar to what this PR is already doing)// interface
eventScrubber: (httpEvent: HttpEvent): => HttpEvent
// in the code:
const defaultEventScrubber = (httpEvent) => {
// some things could already be done by default (e.g. https://docs.sentry.io/security-legal-pii/scrubbing/server-side-scrubbing/event-pii-fields/)
return httpEvent;
};
// ...
constructor(config) {
this.config.eventScrubber = config.eventScrubber ?? defaultEventScrubber
}
// ...
this.context.record(HTTP_EVENT_TYPE, this.config.eventScrubber(httpEvent));Pros
Cons
2. Exposing a configuration (similar to what Sentry docs propose but more TS style)type DataScrub = {
match: RegExp;
replace: string; // what other types? number, boolean??
attributePath: keyof typeof HttpEvent; // would need to be a proper type for a nested path e.g. request.headers.Authorization
}
// interface
eventScrubber: DataScrub[]Implementation is then similar, but done here and not in the consumer's side. Pros
Cons
|
|
I created another branch to start proposing something reusable by all plugins (very WIP to exercise the idea of the interface, basic implementation, etc):
Though I must say that while working on it, "Request URL normalization" was the only real use case I could really think of for HTTP Events. |
Feature proposal
This PR adds a new configuration option to the HTTP telemetry.
This function will be invoked before recording the HTTP event in RUM. And is useful for scenarios like the following:
This helps avoiding noise in the RUM monitor by aggregating data (request count, sessions, etc) about same endpoints in a single "row".
Considerations
I have been running this in a patched version in my project and did not experience any regression issues.
I have considered not exposing this configuration as a function!
Instead it would be receiving URL patterns and what to replace it with. (a similar configuration to
urlsToInclude) to avoid shifting complexity to consumers.But that would be more complexity to be maintained here, and could not be predicting all scenarios. So I followed the idea of the
ignorefunction (from JSError plugin)I have NOT changed all files (documentation, integration tests, etc) because I want to collect feedback for the proposal first.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.