Logs can be viewed via the ipoid dashboard on logstash.
These need a few improvements:
| Title | Reference | Author | Source Branch | Dest Branch | |
|---|---|---|---|---|---|
| update-db: Adjust error logging | repos/mediawiki/services/ipoid!227 | kharlan | main-Ieb75bf687679ac57a26e97c38ed03cd483bc26b3 | main | |
| logging: Switch pipeline logging to winston | repos/mediawiki/services/ipoid!224 | kharlan | main-Ifcc5f1d87852bb7dc6084f27f8f3cca9823daf8d | main | |
| logging: Use winston logger with ECS formatting | repos/mediawiki/services/ipoid!213 | kharlan | main-I2f4167934135b466a03d09a713f4ae763efa1f76 | main | |
| Log at info level instead of debug level | repos/mediawiki/services/ipoid!211 | stran | main-I64b09ff1b146227e95a956048a2d15f4e58bf391 | main |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Declined | None | T351430 Epic: Improve logging for ipoid app | |||
| Declined | None | T355260 Ensure that shell script errors appear in Logstash in ECS format | |||
| Declined | None | T355261 Ensure that timeout errors are caught and logged | |||
| Resolved | kostajh | T357130 Add a run ID for pipeline events | |||
| Declined | None | T358617 Simplify log messages and add more useful information to messages | |||
| Declined | None | T375371 Expose metric for last successful ipoid import date | |||
| Declined | None | T353538 Log unknown actor, risk and tunnel types | |||
| Declined | None | T351746 Include batch file context lines when throwing an error | |||
| Declined | BUG REPORT | STran | T356180 Error: Invalid date passed. Date should be in yyyymmdd format. | ||
| Declined | STran | T355051 Observability: Emit counts in data pipeline | |||
| Declined | None | T355048 Observability: Emit metrics event per request and emit latency per request | |||
| Declined | None | T383770 Improve ipoid's grafana dashboard | |||
| Declined | None | T383898 Instrument data point to view status of `tunnels.anonymous` |
One option would be to remove our dependency on bash scripts; in that case we'd do something like T348479: Consider a CLI framework for ipoid and rewrite the bash scripts as nodejs scripts, which shell out as needed. Then we'd make use of our existing logging library (bunyan) and replace existing usages of console.log with the bunyan logger.
If we want to do something that involves less rewriting, https://sevic.dev/notes/bash-script-json-logging/ has an example of a JSON logger function we could include in the existing shell scripts.
Maybe it makes sense to do both: start with the JSON logger function for bash scripts, but eventually migrate the scripts to nodejs so that they're testable and easier to update and maintain.
kharlan opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/201
logging: Implement structured logging for bash scripts
kharlan opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/202
logging: Use structured logging in node
We're logging too much - e.g. a log line for each batch that is imported (several thousand per day)
I've opted not to do anything about this. A few thousand (structured) entries at an info/debug level don't seem to be a problem, IMHO. Perhaps we could revisit that if we find that it is still too much.
kharlan opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/204
logging: Use pino-pretty with integration script
tchanders merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/201
logging: Implement structured logging for bash scripts
Change 989503 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[operations/deployment-charts@master] ipoid: Bump version
Change 989503 merged by jenkins-bot:
[operations/deployment-charts@master] ipoid: Bump version
dreamyjazz merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/204
logging: Use pino-pretty with integration script
kharlan opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/209
logging: Use pino-pretty with integration script
tchanders merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/202
logging: Use structured logging in node
dreamyjazz merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/209
logging: Use pino-pretty with integration script
Change 989726 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[operations/deployment-charts@master] ipoid: Bump version
Change 989726 merged by jenkins-bot:
[operations/deployment-charts@master] ipoid: Bump version
Logs aren't appearing in Logstash. Trying info level:
https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/210
stran merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/210
Log at info level instead of debug level
stran opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/211
Log at info level instead of debug level
stran closed https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/211
Log at info level instead of debug level
Change 990056 had a related patch set uploaded (by Tchanders; author: Tchanders):
[operations/deployment-charts@master] ipoid: Bump version
Change 990056 merged by jenkins-bot:
[operations/deployment-charts@master] ipoid: Bump version
I think there's one more thing to do in this task, which is to convert the logging format used for requests to make use of https://github.com/pinojs/pino and the elastic common scheme format.
kharlan opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/213
WIP logging: Use pino for express logging
It is working after all, we just need to use the ecs-* data interface, e.g. https://logstash.wikimedia.org/goto/cc0625d83eeea78edcb8ea54602d6eab. Working on converting the existing dashboard to reference that.
stran merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/213
logging: Use winston logger with ECS formatting
Change 995197 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[operations/deployment-charts@master] ipoid: Bump version
Change 995197 merged by jenkins-bot:
[operations/deployment-charts@master] ipoid: Bump version
@jijiki @akosiaris I've deployed that one to staging. Here's an example log entry. Before we move that patch forward to eqiad/codfw, could you please review the metadata we're logging with the request and let us know if there's anything we should be filtering out before we emit a log event?
It looks pretty good to me. Note that the fields with the warning have no mappings in elasticsearch and as such they won't be searchable (e.g. you won't be able to search with a filter of http.response.headers.content-length)
Adding @colewhite so they can override me and object in case I am wrong, but I 'd say feel free to proceed.
P.S. I assume that http.response.body.bytes equaling 404 is coincidental, right?
Sure, that's fine.
Adding @colewhite so they can override me and object in case I am wrong, but I 'd say feel free to proceed.
Ack
P.S. I assume that http.response.body.bytes equaling 404 is coincidental, right?
Yeah I was mildly concerned about that too -- it is a coincidence, see e.g. this request with a different value.
Yeah I was mildly concerned about that too -- it is a coincidence, see e.g. this request with a different value.
😆 😆. Cool, thanks for confirming.
Our schema diverges from upstream slightly to support our needs. "http.response.headers" is one such field that has no upstream equivalent.
The errors you saw are from OpenSearch Dashboards having never encountered this specific "content-length" field.
I refreshed the field mapping - the warnings are gone and the fields are now searchable.
kharlan opened https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/224
logging: Switch pipeline logging to winston
stran merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/224
logging: Switch pipeline logging to winston
kharlan updated https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/227
update-db: Adjust error logging
stran merged https://gitlab.wikimedia.org/repos/mediawiki/services/ipoid/-/merge_requests/227
update-db: Adjust error logging
Change 1002930 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[operations/deployment-charts@master] ipoid: Bump version
Change 1002930 merged by jenkins-bot:
[operations/deployment-charts@master] ipoid: Bump version
Change 1002981 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[operations/deployment-charts@master] ipoid: Bump version
Change 1002981 merged by jenkins-bot:
[operations/deployment-charts@master] ipoid: Bump version
There are still the two subtasks.
I'd like to also clean up the log messages a bit more, but I can file a new subtask for that.
I propose we decline this task and its subtasks in favor of iPoid-Service (IPoid OpenSearch)
As we're no longer certain of the timeline on migrating to OpenSearch, let's improve our logging for our current situation. I'm editing this task to make it an epic since we've got a bunch of small improvement tickets under this topic.