-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Include pipeline and plugin IDs to the JSON logs. #18470
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
Include pipeline and plugin IDs to the JSON logs. #18470
Conversation
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label. Could you fix it @mashhurs? 🙏
|
| generator.writeObjectField("thread", event.getThreadName()); | ||
|
|
||
| final String pipelineId = event.getContextData().getValue("pipeline.id"); | ||
| if (Objects.nonNull(pipelineId) && !pipelineId.isEmpty()) { |
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.
Review note: for the safety I have placed a condition but it exists as we ThreadContext.put("pipeline.id", pipeline_id) and I have confirmed the places we accessing from org.apache.logging.log4j.ThreadContext, example java_pipeline.rb#start
Util::with_logging_thread_context("pipeline.id" => pipeline_id) do
collect_stats
collect_dlq_stats
initialize_flow_metrics
end
Note that, plugin IDs will be placed in the thread context in worker logics.
💛 Build succeeded, but was flaky
Failed CI StepsHistory
cc @mashhurs |
andsel
left a comment
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.
LGTM
* Include pipeline and plugin IDs to the JSON logs. * Add unit tests for plugin and pipeline IDs. (cherry picked from commit 279171b)
* Include pipeline and plugin IDs to the JSON logs. * Add unit tests for plugin and pipeline IDs. (cherry picked from commit 279171b)
* Include pipeline and plugin IDs to the JSON logs. * Add unit tests for plugin and pipeline IDs. (cherry picked from commit 279171b) Co-authored-by: Mashhur <[email protected]>
* Include pipeline and plugin IDs to the JSON logs. * Add unit tests for plugin and pipeline IDs. (cherry picked from commit 279171b) Co-authored-by: Mashhur <[email protected]>
|
@Mergifyio backport 9.2 |
✅ Backports have been created
|
* Include pipeline and plugin IDs to the JSON logs. * Add unit tests for plugin and pipeline IDs. (cherry picked from commit 279171b)
* Include pipeline and plugin IDs to the JSON logs. * Add unit tests for plugin and pipeline IDs. (cherry picked from commit 279171b) Co-authored-by: Mashhur <[email protected]>
Release notes
Include pipeline and plugin IDs to the JSON logs.
What does this PR do?
Adds pipeline and plugin IDs to the JSON logs.
Why is it important/What is the impact to the user?
It crucial to know which plugin from which pipeline is generating a specific logs, especially errors. For now, when using JSON log format with Logstash, it is hard to figure out where the errors (or particular log lines) are coming from.
After this PR, logs provide more visibility for the case which plugin and pipeline are generating the logs.
Examples,
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files (and/or docker env variables)How to test this PR locally
./gradlew clean bootstrap assemble installDefaultGemslogstash.yml, addlog.format: jsonlinebin/logstash -f config/test.confRelated issues
Use cases
Screenshots
Logs