logging: update MozLogFormatter to output MozLog (bug 1977738)#834
logging: update MozLogFormatter to output MozLog (bug 1977738)#834
Conversation
|
View this pull request in Lando to land it once approved. |
shtrom
force-pushed
the
bug1977738/MozLogFormatter
branch
2 times, most recently
from
4d6b12c to
828545d
Compare
shtrom
force-pushed
the
bug1977738/MozLogFormatter
branch
2 times, most recently
from
8f904d8 to
8533245
Compare
shtrom
changed the title
shtrom
force-pushed
the
bug1977738/MozLogFormatter
branch
from
8533245 to
e4a1720
Compare
shtrom
force-pushed
the
bug1977738/MozLogFormatter
branch
from
e4a1720 to
156756f
Compare
| STATIC_URL = os.getenv("STATIC_URL") | ||
|
|
||
| # Defined in lando.settings. | ||
| STATIC_URL = os.getenv("STATIC_URL", STATIC_URL) # noqa: F405 |
There was a problem hiding this comment.
Not clear why we are doing this here. All remote environments should set their own value here, vs. using what's in lando.settings. Can you add a comment explaining the reasoning behind this?
There was a problem hiding this comment.
I added this comment to the code:
If missing, we default to
STATIC_URL, from thefrom settings import *.
This allows the code to run in a local environment configured to simulate a remote
environment.
Alternatively, we could add a commented-out definition of STATIC_URL in the compose.yaml file alongside ENVIRONMENT and DJANGO_SETTINGS_MODULE, but I thought it was better to make the issue self-correcting.
There was a problem hiding this comment.
nit: I think this could be better handled, given that we may handle this stuff with a make command or other type of script. I.e., we could set this value explicitly in the environment when in "simulate remote environment" mode without needing to have a special case for this value here.
zzzeid
left a comment
zzzeid
left a comment
There was a problem hiding this comment.
See my comment on mozilla-conduit/suite#77 RE: maybe adding this to a make command instead. I think there is value in documenting this, or making it more easily switchable, but I think we might as well add a make command for this. I assume updating the .env or even an environment variable directly for may be sufficient?
Ok, I think this change is better suited for suite (no pun intended) anyway. I removed the compose.yaml changes from here, and then we can iterate on mozilla-conduit/suite#77 |
| STATIC_URL = os.getenv("STATIC_URL") | ||
|
|
||
| # Defined in lando.settings. | ||
| STATIC_URL = os.getenv("STATIC_URL", STATIC_URL) # noqa: F405 |
There was a problem hiding this comment.
nit: I think this could be better handled, given that we may handle this stuff with a make command or other type of script. I.e., we could set this value explicitly in the environment when in "simulate remote environment" mode without needing to have a special case for this value here.
|
Pull request closed by commit 12f2665 |
…han,zeid There are two MozLog standards 1. https://wiki.mozilla.org/Firefox/Services/Logging#MozLog_application_logging_standard 2. https://firefox-source-docs.mozilla.org/mozbase/mozlog.html#data-format Up to now, lando was outputing the former, but this wasn't formatted very nicely in the GCP Log Explorer. Instead, modifying the `MozLogFormatter` to output data in a format compatible with the latter works much better. For the time being, the originally-formatted data is retained, as it doesn't impact the formatting in the Logs Explorer. --- * logging: update MozLogFormatter to output MozLog (bug 1977738) * landing_worker: fix log message typo * worker: log Worker.name when paused * dockerflow: log heartbeat requests at debug level Pull request: #834
3 participants
There are two MozLog standards
Up to now, lando was outputing the former, but this wasn't formatted very nicely in the GCP Log Explorer.
Instead, modifying the
MozLogFormatterto output data in a format compatible with the latter works much better.For the time being, the originally-formatted data is retained, as it doesn't impact the formatting in the Logs Explorer.