-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
chore(pelias_parser): add logging for chosen parser solution scores #1381
base: master
Are you sure you want to change the base?
Conversation
@@ -95,6 +95,11 @@ function parse (clean) { | |||
// ' VVVV NN SSSSSSS AAAAAA PPPPP ' | |||
let mask = solution.mask(t); | |||
|
|||
// log information about the selected solution | |||
logger.info('pelias_parser_solution', { | |||
score: solution.score.toFixed(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be interesting to have the input too here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The input is already being logged at
api/sanitizer/_text_pelias_parser.js
Line 66 in ef2969b
params: clean, |
I decided to split up those log sections so the pelias_parser
log entry provides an overview of the parse and then we can have one or more pelias_parser_solution
lines which log info about specific solutions.
At this stage, we only pick the first solution, but we could expand this later and if we end up having multiple pelias_parser_solution
log lines it'll be cleaner to have the input only logged once since it doesn't change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to have a simple way to analyze the logs.
For example if we want to quickly know which queries have a low score and the associated input all at once (using kibana or whatever). Unless it is possible to have links between log lines ? 🤔
It sounds like the purpose of this code is for manual investigation into a single query at a time, rather than providing data that we could analyze across many requests. In that case, it might be better as part of the debug output, since it would then be along side all the other context from a request. If we are trying to log something for later analysis, we'll need to do as @Joxit suggests and associate the solution score with a bunch of other data like the input text, or it won't be very useful. |
When I was writing up pelias/parser#76 today I noticed that the 'scoring' in the Pelias parser is pretty good, it's fairly indicative of when the parse was low quality and you'll see scores <
0.5
next to a lot of incorrect parses 🎉So this PR is pretty simple, it adds logging of the solution scores so that we can graph them and get a better idea about how it's doing in an unpredictable production environment.
example log line: