DM-34020: Get Google Cloud to detect logging levels in prompt_prototype #26
Conversation
kfindeisen
force-pushed
the
tickets/DM-34020
branch
3 times, most recently
from
1927180 to
7961443
Compare
| entry = { | ||
| "severity": record.levelname, | ||
| "logging.googleapis.com/labels": self._labels, | ||
| "message": record.getMessage(), | ||
| } |
There was a problem hiding this comment.
Hey, can we optimize the create of the entry by only adding labels if we have them as I've checked google docs and they seem to be optional? Do we need this label for something else? Let me know your thoughts.
entry = {
"severity": record.levelname,
"message": record.getMessage(),
}
if self._labels:
entry["logging.googleapis.com/labels"] = self._labels
There was a problem hiding this comment.
Ironically, I had a commit like that, which I dropped as unnecessary complexity. 😕
No, I don't think we need the labels for anything else; my reasoning was that any "labels": {} in the output would just get silently dropped by Google, so there was no need to clean them out.
There was a problem hiding this comment.
Ah, alright, that makes sense. I had just been thinking that, if this was run often and the labels were often empty, it would slow things down when it didn't need to. Would we expect the number of times this gets called to make create empty labels and dropping them have a noticeable effect?
There was a problem hiding this comment.
Probably not? The activator code adds an instrument label now, and I'd expect any future evolution to be towards more labels, not less.
GCloudStructuredLogFormatter is currently functionally identical to the original call to basicConfig, but will be easier to modify in the future (in particular, easier to replace once we migrate away from Google Cloud). (And no, I don't know why Google doesn't already provide a class for this.)
This parameter is currently unused, but will allow parsing that can't use the existing fmt argument.
Using the JSON serialization code directly avoids special-case errors like quotes, and makes it easier to extend the log format to more fields.
The implementation of format now uses a hardcoded representation of the Google Cloud structured format.
Using JSON in the test suite eliminates any possible errors from writing out the serialized form by hand.
This lets the warnings be correctly parsed by Google code.
The "fatal" Python logging level is translated to Google's "critical", which is defined as "cause more severe problems or outages". While it's certainly bad if a visit can't be processed at all, it won't affect the rest of the system. Google's "error" is defined merely as "likely to cause problems", and is more appropriate.
kfindeisen
force-pushed
the
tickets/DM-34020
branch
from
7961443 to
3c48ef4
Compare
3 participants
This PR fixes several bugs that together prevented some Prompt Processing logs from being correctly parsed by Google Cloud. In the process, it factors out the existing logging setup into a form that will be easier to expand later (e.g., with more context-specific information).
With these changes, the only output that is still unparsed is that from Google infrastructure itself (e.g., worker startup/shutdown) and the "Overriding default configuration file with [path]/config/.dustmapsrc" message, which appears to be sent directly to standard output.