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

[5889] Bmoric/mdc manipulation tools [1/2] #7266

Merged
merged 5 commits into from
Oct 22, 2021

Conversation

benmoriceau
Copy link
Contributor

@benmoriceau benmoriceau commented Oct 21, 2021

What

This is preparing the tagging of the application that generate a log which is requested in #5889

It is adding helper to add a color to the log and safely manipulate the MDC.

It is also modifiying the log4j pattern.

The use of those helper will will be implemented in another PR.

@benmoriceau benmoriceau added area/platform issues related to the platform 2021-q4-platform labels Oct 21, 2021
@benmoriceau benmoriceau changed the title Bmoric/mdc manipulation tools [7182] Bmoric/mdc manipulation tools [1/3] Oct 21, 2021
@benmoriceau benmoriceau changed the title [7182] Bmoric/mdc manipulation tools [1/3] [5889] Bmoric/mdc manipulation tools [1/3] Oct 21, 2021

@BeforeEach
public void init() {
MDC.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MDC.clear();

From the MDC docs:

Set the current thread's context map by first clearing any existing map and then copying the map passed as parameter

It shouldn't be necessary to clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done,

CYAN("\u001b[36m"),
WHITE("\u001b[37m");

private final String ainsiCode;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ansi, not ainsi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


@Override
public void close() throws Exception {
MDC.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MDC.clear();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -4,7 +4,7 @@

<!-- The following patterns mask the string apikey=<string> to apikey=***** to prevent secrets leaking. -->
<Property name="default-pattern">%d{yyyy-MM-dd HH:mm:ss}{GMT+0} %highlight{%p} %C{1.}(%M):%L - %X - %replace{%m}{apikey=[\w\-]*}{apikey=*****}%n</Property>
<Property name="docker-worker-file-pattern">%d{yyyy-MM-dd HH:mm:ss}{GMT+0} %p (%X{job_root}) %C{1}(%M):%L - %replace{%m}{apikey=[\w\-]*}{apikey=*****}%n</Property>
<Property name="docker-worker-file-pattern">%replace{%X{log_source} - }{^ - }{}%d{yyyy-MM-dd HH:mm:ss}{GMT+0} %p (%X{job_root}) %C{1}(%M):%L - %replace{%m}{apikey=[\w\-]*}{apikey=*****}%n</Property>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment to describe what's happening here? It's kind of difficult to work backwards to figure out what is happening here for someone not super familiar with log4j2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* </code>
* </pre>
*/
public class ScopedMDCChange implements AutoCloseable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't really love the name. Maybe MdcScope? Definitely not blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@benmoriceau benmoriceau temporarily deployed to more-secrets October 22, 2021 00:05 Inactive
Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!


/**
* This class is an autoClosable class that will add some specific values into the log MDC. When
* being close, it will restore the orginal MDC. It is advise to use it like that:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* being close, it will restore the orginal MDC. It is advise to use it like that:
* being closed, it will restore the original MDC. It is advised to use it like that:

@@ -4,7 +4,8 @@

<!-- The following patterns mask the string apikey=<string> to apikey=***** to prevent secrets leaking. -->
<Property name="default-pattern">%d{yyyy-MM-dd HH:mm:ss}{GMT+0} %highlight{%p} %C{1.}(%M):%L - %X - %replace{%m}{apikey=[\w\-]*}{apikey=*****}%n</Property>
<Property name="docker-worker-file-pattern">%d{yyyy-MM-dd HH:mm:ss}{GMT+0} %p (%X{job_root}) %C{1}(%M):%L - %replace{%m}{apikey=[\w\-]*}{apikey=*****}%n</Property>
<!-- The following patter log the application name in the beginning of the line, then a regular log line after a - separator. If the application name is not present in the context, the separator is removed -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<!-- The following patter log the application name in the beginning of the line, then a regular log line after a - separator. If the application name is not present in the context, the separator is removed -->
<!-- The following pattern log the application name in the beginning of the line, then a regular log line after a - separator. If the application name is not present in the context, the separator is removed -->

@benmoriceau benmoriceau changed the title [5889] Bmoric/mdc manipulation tools [1/3] [5889] Bmoric/mdc manipulation tools [1/2] Oct 22, 2021
@benmoriceau benmoriceau merged commit a616c29 into master Oct 22, 2021
@benmoriceau benmoriceau deleted the bmoric/mdc-manipulation-tools branch October 22, 2021 22:07
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
This is preparing the tagging of the application that generate a log which is requested in airbytehq#5889

It is adding helper to add a color to the log and safely manipulate the MDC.

It is also modifying the log4j pattern.

The use of those helper will will be implemented in another PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants