Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(spans): Group by domain and last segment of the path #2654

Merged

Conversation

phacops
Copy link
Contributor

@phacops phacops commented Oct 25, 2023

Instead of trying to group by the full path, since it's containing a whole lot of cardinality, we'll isolate the last part of the path (which is usually the filename of the resource but can also be a part of the path or empty if the URL finishes with a /), scrub this part only and construct a group based on the scrubbed domain and this scrubbed last part.

@jjbayer I don't think the logic meaning to add a dummy base URL to relative or absolute paths is working as intended. In the case of domains without a scheme, the dummy domain is added and is now part of the path, which means it's scrapped with this PR. I haven't figured out a proper solution to this issue yet. Ideally, we'd identify the scheme is empty and use a dummy scheme or something.

@phacops phacops requested review from a team and jjbayer October 25, 2023 01:44
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

Ideally, we'd identify the scheme is empty and use a dummy scheme or something.

@phacops we could use a dummy domain for paths that start with /, and for paths that don't, use a dummy scheme instead.

@phacops
Copy link
Contributor Author

phacops commented Oct 25, 2023

Let's not handle that case as I'm not sure it's a real situation yet. It'll go to the relative path logic and ends up with a wildcard and the filename.

@phacops phacops enabled auto-merge (squash) October 25, 2023 16:32
@phacops phacops merged commit 01e36c9 into master Oct 25, 2023
20 checks passed
@phacops phacops deleted the pierre/spans-group-resource-by-domain-filename-extension branch October 25, 2023 18:15
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.

None yet

2 participants