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

Enable production-like logs if env variable is set #302

Merged
merged 3 commits into from
Jul 5, 2023

Conversation

lauraghiorghisor-tw
Copy link
Contributor

This is to allow setting production-like behaviour by using the ENABLE_PRODUCTION_LIKE_LOGS env variable. This will be set in govuk-heml-charts -> env-configmap.yaml. The purpose is to allow developers to override this env var in order to test changes to production logs locally.

Trello card

Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

I've made a suggestion to the variable naming.

We'll also need to add an entry to the change log. With semantic versioning we should consider this a breaking change, which would seem a bit of a pain but given #300 already has breaking changes we can release them together and not need two major versions.

I'll leave a note on #300 to say to be careful with their version bump.

lib/govuk_app_config/railtie.rb Outdated Show resolved Hide resolved
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Looks good, I've made a couple of suggestions.

Thanks for GovukJsonLogging update ⭐

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
This is to allow setting production-like behaviour by using an env
variable. This env variable will be set in govuk-heml-charts repo.
The purpose is to allow developers to override this env var in order
to test changes to production logs locally.
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks. I guess we can't release this until we've got the helm chart change applied to each env

README.md Show resolved Hide resolved
@lauraghiorghisor-tw lauraghiorghisor-tw merged commit b713113 into main Jul 5, 2023
5 checks passed
@lauraghiorghisor-tw lauraghiorghisor-tw deleted the enable-production-like-logging branch July 5, 2023 09:16
@floehopper
Copy link

floehopper commented Jul 10, 2023

I guess we can't release this until we've got the helm chart change applied to each env

@lauraghiorghisor-tw Can I double-check that alphagov/govuk-helm-charts#1190 adds the ENABLE_PRODUCTION_LIKE_LOGS env var to all apps and so it's safe to merge the latest version of this gem into an app like signon?

@lauraghiorghisor-tw
Copy link
Contributor Author

I guess we can't release this until we've got the helm chart change applied to each env

@lauraghiorghisor-tw Can I double-check that alphagov/govuk-helm-charts#1190 adds the ENABLE_PRODUCTION_LIKE_LOGS env var to all apps and so it's safe to merge the latest version of this gem into an app like signon?

@floehopper Just saw this now, my apologies. Thank you for confirming this with someone else and bumping the version accordingly. 🙏🏻

Also, we identified a small issue that affects development, you might want to bump the version again. See PR here.

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

4 participants