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

enhance(glean): add http_status metric to pings #8420

Merged
merged 2 commits into from
Mar 15, 2023
Merged

enhance(glean): add http_status metric to pings #8420

merged 2 commits into from
Mar 15, 2023

Conversation

LeoMcA
Copy link
Member

@LeoMcA LeoMcA commented Mar 14, 2023

Summary

https://mozilla-hub.atlassian.net/browse/MP-285
data review (approved): https://bugzilla.mozilla.org/show_bug.cgi?id=1822124

Problem

We can't filter 404s out of our glean data, nor filter just by them to understand what popular 404s are.

Solution

Add a http_status metric for the current page, currently only either "200" or "404"

Merged HydrationData types, and typed pageNotFound property. I've put properly typing the other props on my todo list, which I'll do in another PR.


How did you test this change?

Had to test on :5042, as :3000 doesn't SSR and therefore doesn't set the appProps.pageNotFound property.

To get glean working on :5042, either do some git fu to incorporate #8383, or just define const CRUD_MODE = true; at the top of glean-context.tsx.

Example ping: https://debug-ping-preview.firebaseapp.com/pings/mdn-dev/d7d33402-2ea4-4b72-b8a2-ef85e2d1dacf#L38

currently only either "200" or "404"

merged HydrationData types, and typed pageNotFound property
@LeoMcA LeoMcA requested a review from caugner March 14, 2023 17:56
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, just some comments.

@caugner caugner merged commit 2c7231e into main Mar 15, 2023
@caugner caugner deleted the glean-404 branch March 15, 2023 21:19
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