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

Remove Metrics Data Page #3744

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented May 29, 2024

This PR removes the old generation files for the metrics data, as they are no longer used. Current metric generation files are in /ansible/roles/metrics/.

CC @nodejs/build-files
CC @nodejs/web-infra

@flakey5
Copy link
Member

flakey5 commented May 29, 2024

I'm for adding a redesigned metrics page (context) but I think it should stay as-is on nodejs.org until that page is ready to be deployed. Since the release worker is serving /metrics, this pr won't change that behavior.

I'm not sure if it'd make more sense to serve a redesigned metrics page from the release worker still or move it to the website repo though. I think it'd be preferrable to have it in the latter? cc @ovflowd

@avivkeller
Copy link
Member Author

but I think it should stay as-is on nodejs.org until that page is ready to be deployed.

That's probably a good idea, (hence this is a draft)

@targos
Copy link
Member

targos commented May 29, 2024

If the setup here is effectively unused, we can remove it regardless of the new page being ready or not.

@avivkeller
Copy link
Member Author

If the setup here is effectively unused, we can remove it regardless of the new page being ready or not.

The setup is unused, but exists for historical reasons. The historical data is not up to date, or in the same format as the current data.

@flakey5
Copy link
Member

flakey5 commented May 29, 2024

If the setup here is effectively unused, we can remove it regardless of the new page being ready or not.

+1

@avivkeller avivkeller changed the title (Discuss) Remove Metrics Data Page Remove Metrics Data Page May 29, 2024
@avivkeller avivkeller marked this pull request as ready for review May 29, 2024 15:45
@avivkeller
Copy link
Member Author

Seeing as both of you agree that this data is effectively unused, I've marked this as ready for review, so that it can be merged when ready

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

I don't think we should land this until a replacement is in place and https://nodejs.org/metrics/ redirected to the replacement page.

There is value in the current https://nodejs.org/metrics/ being a placeholder page that says "this is historical data, for current data see this publicly accessible GCP bucket".

@richardlau
Copy link
Member

To clarify a bit, perhaps some of the behind-the-scenes generation/processing stuff could be removed now, but we'd want to keep whatever is serving the current https://nodejs.org/metrics/ until it is replaced.

@targos
Copy link
Member

targos commented May 29, 2024

A replacement is already in place. https://nodejs.org/metrics/ is served from Cloudflare and doesn't reach the www server.

@richardlau
Copy link
Member

A replacement is already in place. https://nodejs.org/metrics/ is served from Cloudflare and doesn't reach the www server.

Where is the html it is serving coming from?

@flakey5
Copy link
Member

flakey5 commented May 29, 2024

Where is the html it is serving coming from?

The dist-prod R2 bucket on the Node.js cf account, same with the actual log files. Served by the release worker

@richardlau richardlau dismissed their stale review May 29, 2024 16:22

Outdated

@MattIPv4
Copy link
Member

I would be +1 for removing metrics-related stuff from this repo as this PR is doing, given the scripts aren't running and NGINX isn't serving the content.

nodejs/release-cloudflare-worker#120 can then act as the discussion for building an actual replacement within https://github.com/nodejs/nodejs.org/ before https://nodejs.org/metrics/ is removed from the internet itself.

@avivkeller
Copy link
Member Author

avivkeller commented May 29, 2024

BTW some scripts still exist in this repo. /ansible/roles/metrics/ has scripts, but I'm unsure whether they are in use

@bmuenzenmeyer
Copy link

this issue seems relevant to discussion here, but i only had a glance nodejs/nodejs.org#6822

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.

7 participants