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

Delete `govuk_cdnlogs` code as it's superseded by AWS S3 and Athena #10126

Merged
merged 1 commit into from Feb 19, 2020

Conversation

@issyl0
Copy link
Member

issyl0 commented Feb 6, 2020

This is draft because a) I'm not sure I've got everything and b) should we do this?

@issyl0 issyl0 force-pushed the rm-govuk-cdn-logs-manual-process branch from 689a76b to 62a048f Feb 7, 2020
@issyl0 issyl0 marked this pull request as ready for review Feb 7, 2020
@issyl0 issyl0 requested a review from sengi Feb 7, 2020
@sengi
sengi approved these changes Feb 10, 2020
Copy link
Contributor

sengi left a comment

SGTM. Do any of these cronjobs and things need to be removed from the machine or are they already gone?

@issyl0

This comment has been minimized.

Copy link
Member Author

issyl0 commented Feb 10, 2020

They do probably still exist. I'll re-make this with some ensure => absent tomorrow, unless you know which ones they are?

@sengi

This comment has been minimized.

Copy link
Contributor

sengi commented Feb 10, 2020

They do probably still exist. I'll re-make this with some ensure => absent tomorrow, unless you know which ones they are?

I don't know for sure. ensure => absent SGTM. (If you prefer to just remove stuff by hand then that's fine with me, too - whatever you think is easiest.)

Thanks for tidying this up btw 🌟

@issyl0 issyl0 force-pushed the rm-govuk-cdn-logs-manual-process branch 2 times, most recently from 5daa63c to 67e0104 Feb 11, 2020
@issyl0

This comment has been minimized.

Copy link
Member Author

issyl0 commented Feb 11, 2020

I deployed this branch to integration and it seems to be fine. 👍

@kevindew

This comment has been minimized.

Copy link
Member

kevindew commented Feb 14, 2020

In the RFC I remember there were people who keen to keep the sending of logs to our monitoring machines. There is a timing advantage to those sent to monitoring machine. I believe they are streamed at something close to real time via logstash, whereas the athena monitoring has a delay of between 15 and 30 minutes for data.

I don't really think many people (if anyone) really uses the ones from the monitoring machine, but it also feels like the sort of thing where it could be not frustrating not to have in the midst of an incident.

@issyl0

This comment has been minimized.

Copy link
Member Author

issyl0 commented Feb 18, 2020

@kevindew Thanks for that. As we discussed in Slack, only about three people who have been here long enough would remember that there are also useable files on the monitoring machine - indeed all the incident docs point to Athena. I'm not sure what we should do here, but you raise valid points. 🤔

@kevindew

This comment has been minimized.

Copy link
Member

kevindew commented Feb 18, 2020

@kevindew Thanks for that. As we discussed in Slack, only about three people who have been here long enough would remember that there are also useable files on the monitoring machine - indeed all the incident docs point to Athena. I'm not sure what we should do here, but you raise valid points. 🤔

Yup, can see it's not ideal to be caught in limbo on how to move forward.

Given we think that a) no-one uses these files, b) only a few people know about them and c) we've done the work to remove them; then I think we should proceed with the removal but leave some information in the docs about their past existence, to help us if we decide we want them back.

So I'd suggest perhaps changing the second paragraph of https://docs.publishing.service.gov.uk/manual/query-cdn-logs.html#header to instead reference that we no longer have logs under 15 minutes due to low usage and link to this PR as a means to restore them if we need to.

How does that sound?

issyl0 added a commit to alphagov/govuk-developer-docs that referenced this pull request Feb 19, 2020
…onitoring`

- Most of the people who used to use these real-time logs have left.
- Athena is good enough for most people's purposes.
- We can clear up a lot of code (and save ourselves money) if we stop
  storing them in two places.
- It's able to be reverted though, if we know in advance that we're
  going to need instant logs, or if they're deemed useful again in the
  future.

Further discussion: alphagov/govuk-puppet#10126 and #2285 (comment)
@issyl0

This comment has been minimized.

Copy link
Member Author

issyl0 commented Feb 19, 2020

Sounds good, thanks @kevindew! I've done that in alphagov/govuk-developer-docs#2331.

issyl0 added a commit to alphagov/govuk-developer-docs that referenced this pull request Feb 19, 2020
…ring`

- Most of the people who used to use these real-time logs have left.
- Athena is good enough for most people's purposes.
- We can clear up a lot of code (and save ourselves money) if we stop
  storing them in two places.
- It's able to be reverted though, if we know in advance that we're
  going to need instant logs, or if they're deemed useful again in the
  future.

Further discussion: alphagov/govuk-puppet#10126 and #2285 (comment)
@issyl0 issyl0 force-pushed the rm-govuk-cdn-logs-manual-process branch from 67e0104 to cea20ed Feb 19, 2020
@issyl0 issyl0 requested a review from kevindew Feb 19, 2020
- Pulling these from Fastly into files on disk was superseded in [2018](alphagov/govuk-rfcs#93).
- This code and its docs were rediscovered in alphagov/govuk-developer-docs#2285 (comment) and it was confusing.
@issyl0 issyl0 force-pushed the rm-govuk-cdn-logs-manual-process branch from cea20ed to 85e6f6a Feb 19, 2020
@issyl0 issyl0 merged commit b968631 into master Feb 19, 2020
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@issyl0 issyl0 deleted the rm-govuk-cdn-logs-manual-process branch Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.