Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Fix unstructured logging variable formatting #2029
Fix unstructured logging variable formatting #2029
Changes from 3 commits
9d7d4c5
2c6623c
b10f0e5
a2339ae
114feb0
43eff03
0c60b41
a53c2f1
39f26a2
7262d29
8033bd6
43e1985
68a56d8
49af0b8
539f0de
3bb80a9
1ae613c
4d7a6d0
bd89a30
d085954
b35a91e
3ddd35d
18f40af
29c927b
fc2fa46
d0ef8aa
dd9e5fc
5a27f33
6db7d61
5e04164
89e7245
e050a51
37122f4
e8631f8
7a31f8e
79b1ede
f8220a5
3d44e67
a3d0aab
0b8d527
10df87a
9947332
dcdbed8
b0c31ae
0fb33fe
bc6a1dc
54d183f
7c7f15a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Match the above?
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.
I wasn't sure since the original log wanted to capture it as a signal and not into seperate metric, sensor, geo....
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.
Yea, these old pipelines like to mix together geo resolution, metric and sensor name into a single name and call it signal, though that doesn't match our definition of signal anywhere else. Seems better to me to separate these out, since then you can do granular search in the logs without doing substring search.
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.
Two lines below this is a call to
create_export_csv()
which combines metric and sensor in the output file name, in what will ultimately be the signal name. How about:Should we use this opportunity to add better logging to
create_export_csv()
? @aysim319, you have something planned to change doctor visits and hospital admissions to use this method, right? Perhaps we can add the logging then.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.
yeah.. I think it's better to revisit once all the indicators are using create_export_csv... because at least for hospital-admissions it doesn't quite use the metric and the sensor combination if I recall