Skip to content
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

Add job id to all batch-job-specific Workflow Manager log messages #1341

Closed
jrobble opened this issue May 28, 2021 · 2 comments
Closed

Add job id to all batch-job-specific Workflow Manager log messages #1341

jrobble opened this issue May 28, 2021 · 2 comments
Assignees
Milestone

Comments

@jrobble
Copy link
Member

jrobble commented May 28, 2021

Note: Prior to adding the job id to new log messages, and changing the job id format, understand the impact on end-user parsers and visualizations.

Update the WFM to use a Mapped Diagnostic Context (MDC) for reporting the Job Id for all job-specific log lines. Currently, many log lines lack the job id since it's burden to have to pass it to every method in every class. MDC gets around this.

We can modify the layoutPattern in openmpf/trunk/workflow-manager/src/main/resources/log4j2.xml:

<Property name="layoutPattern">%date %level [%thread] %logger{1.}%notEmpty{ [Job %X{jobId}]} - %msg%n</Property>

Here's an example of changes that need to be made to the Java code:

package org.mitre.mpf;

import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;

public class TestMdc {

    private static final Logger LOG = LoggerFactory.getLogger(TestMdc.class);

    @Test
    public void test() {
        var jobId = 1234;
        try (var ctx = MDC.putCloseable("jobId", String.valueOf(jobId))) {
            doesNotKnowJobId();
            existingMethodThatAlreadyLogsJobId(jobId);
        }
        LOG.info("Message not related to a specific job.");
    }

    private static void doesNotKnowJobId() {
        LOG.info("hello");
    }

    private static void existingMethodThatAlreadyLogsJobId(int jobId) {
        LOG.info("Doing something for Job id {}", jobId);
    }
}

Output:

2021-05-28 15:18:37,123 INFO [main] o.m.m.TestMdc [Job 1234] - hello
2021-05-28 15:18:37,126 INFO [main] o.m.m.TestMdc [Job 1234] - Doing something for Job id 1234
2021-05-28 15:18:37,126 INFO [main] o.m.m.TestMdc - Message not related to a specific job.

If we just put it in WfmProcessor and WfmSplitter, we will have it applied to the majority of the log messages with only a few lines of code.

Also, note that currently we have some WFM messages that generate messages with job information like [Job 864|*|*], where the asterisks are intended to represent the task and action id. There are only 8 non-debug places in the code where they are provided with actual values and not just *. We should consider removing the |*|* part of the log messages altogether if no one uses that information.

Ensure that the media inspection work queue log job ids for all messages (should be handled by above).

It may make the most sense to complete #1282 at the same time as this task.

@jrobble jrobble added this to the Milestone 2 milestone May 28, 2021
@jrobble jrobble self-assigned this May 28, 2021
@jrobble jrobble added this to To do in OpenMPF: Development via automation May 28, 2021
@jrobble jrobble moved this from To do to Planned in OpenMPF: Development May 28, 2021
@jrobble
Copy link
Member Author

jrobble commented May 28, 2021

Currently blocked pending more information about the impact on end-user parsers and visualizations.

@jrobble jrobble changed the title Add job id to all job-specific Workflow Manager log messages and other improvements Add job id to all job-specific Workflow Manager log messages Jun 11, 2021
@jrobble
Copy link
Member Author

jrobble commented Jul 14, 2021

Unblocked: We got the go-ahead because we're planning on putting the job id immediately after the hypen before the log message.

@jrobble jrobble removed the blocked label Jul 14, 2021
@jrobble jrobble assigned brosenberg42 and unassigned jrobble Jul 14, 2021
@jrobble jrobble moved this from Planned to In Progress in OpenMPF: Development Jul 14, 2021
@jrobble jrobble changed the title Add job id to all job-specific Workflow Manager log messages Add job id to all batch-job-specific Workflow Manager log messages Aug 31, 2021
@jrobble jrobble closed this as completed Sep 8, 2021
OpenMPF: Development automation moved this from In Progress to Closed Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants