You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/04/01 08:16:04 UTC
[kudu-CR] wip ranger: direct client logs to a log file
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15628
Change subject: wip ranger: direct client logs to a log file
......................................................................
wip ranger: direct client logs to a log file
This patch enables us to direct Java subprocess logs to a rolling file
with the new "-r" option. The subprocess will use the KUDU_LOG_DIR and
KUDU_LOG_NAME system properties to output logs as appropriate.
Since, in production, we expect users to prefer these logs over stdout,
this also makes logging to stdout optional with the new "-s" option.
Logging to stdout is enabled in subprocess_proxy-test and
subprocess_server-test since it's useful to view the subprocess logs
in-line with test output.
wip: needs a test
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/echo/EchoSubprocessMain.java
M java/kudu-subprocess/src/main/resources/log4j2.properties
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/subprocess/subprocess_server-test.cc
6 files changed, 82 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/1
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] ranger: direct client logs to a log file
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: direct client logs to a log file
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/2/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/2/src/kudu/ranger/ranger_client.cc@189
PS2, Line 189: Substitute("-DKUDU_LOG_DIR=$0", FLAGS_log_dir),
: Substitute("-DKUDU_LOG_NAME=$0", log_filename),
Can you add a comment here to explain what these are about? Also what is '-r'.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 Apr 2020 04:08:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: ranger: enable log4j2 logging to files
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 11
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@11
PS7, Line 11: logging
nit: Maybe just `log`
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@21
PS7, Line 21: ranger_enable_debug_logging
Would it make sense to do `--ranger_log_level` and accept standard log4j levels and default to INFO? (TRACE,DEBUG,INFO,WARN,ERROR)
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@77
PS7, Line 77: client
nit: should we remove `client`. This can contain subprocess logs too right?
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@81
PS7, Line 81: TAG_FLAG(ranger_logging_config_dir, advanced);
Should we mark this as evolving?
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@85
PS7, Line 85: TAG_FLAG(ranger_enable_debug_logging, advanced);
Should we mark this as evolving?
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@89
PS7, Line 89: TAG_FLAG(ranger_logtostdout, hidden);
Should we also mark this as `unsafe` since it's for testing only?
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 14:49:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@14
PS8, Line 14: on
> one
Done
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@13
PS8, Line 13: This means a file will not be regenerated
: if on has already been generated
> One downside to this approach is that we can't automatically update log4j2.
Yeah, I will likely default the "overwrite" flag in a later patch to 'true'.
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@17
PS8, Line 17: directory
> Nit: the directory
Done
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@255
PS8, Line 255: if (!env->FileExists(log_conf_dir)) {
: RETURN_NOT_OK(env->CreateDir(log_conf_dir));
: }
> I don't think we should be creating FLAGS_log_dir. Maybe condition this on
I'd rather not add more work for admins, so I'll condition on using an explicit config dir.
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296
PS8, Line 296: -Dlog4j2.configurationFile
> Nit: maybe -Dlog4j2.configuration.filename?
That doesn't seem to work (it's not honored by log4j2 at least). Leaving this as is.
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/subprocess/subprocess_proxy.cc
File src/kudu/subprocess/subprocess_proxy.cc:
PS8:
> We're doing more and more of this "template composition", and it's getting
Yeah, I left a TODO complaining about this. Builder, mustache, whatever is easy to write would get points in my book.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 00:14:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#7).
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the file already exists, the
existing file is honored.
This properties file will direct logs to directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and log42j debug-level logging to be
useful, so this also adds --ranger_logtostdout and
--ranger_enable_debug_logging for these respectively. I opted not to
use the glog --minloglevel flag since they don't quite map to log4j2 log
levels (e.g. glog doesn't have debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005349.0.5236
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005609.0.5320
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005921.0.5395
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005349.5236
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005609.5320
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395
kudu-ranger-client-log4j2.properties
kudu-ranger-client.awong-MBP16-21621.local.20200403-005349.log.gz
kudu-ranger-client.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/gradle/dependencies.gradle
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
8 files changed, 295 insertions(+), 60 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/7
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 11: Verified+1
Unrelated flake.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 11
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 20:21:05 +0000
Gerrit-HasComments: No
[kudu-CR] wip ranger: direct client logs to a log file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: wip ranger: direct client logs to a log file
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/1//COMMIT_MSG@10
PS1, Line 10: The subprocess will use the KUDU_LOG_DIR and
: KUDU_LOG_NAME system properties to output logs as appropriate.
Could you add a sample listing of the log directory and the subprocess log files in it?
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 01 Apr 2020 18:56:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: direct client logs to a log file
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: direct client logs to a log file
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/2/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/2/src/kudu/ranger/ranger_client.cc@189
PS2, Line 189: Substitute("-DKUDU_LOG_DIR=$0", FLAGS_log_dir),
: Substitute("-DKUDU_LOG_NAME=$0", log_filename),
> Can you add a comment here to explain what these are about? Also what is '-
Done
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 Apr 2020 04:31:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: direct client logs to a log file
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#4).
Change subject: ranger: direct client logs to a log file
......................................................................
ranger: direct client logs to a log file
This patch enables us to direct Java subprocess logs to a rolling file
with the new "-r" option. The subprocess will use the KUDU_LOG_DIR and
KUDU_LOG_NAME system properties to output logs as appropriate.
Since, in production, we expect users to prefer these logs over stdout,
this also makes logging to stdout optional with the new "-s" option.
Logging to stdout is enabled in subprocess_proxy-test and
subprocess_server-test since it's useful to view the subprocess logs
in-line with test output.
Here's what's in a Ranger-configured Kudu master's log directory (I
lowered the rolling size threshold for the sake of generating a new roll
quickly):
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.WARNING -> kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200401-193145.0.59433
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-ranger-client.awong-MBP16-21621.local.20200401-193145.log.gz
kudu-ranger-client.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
M java/kudu-subprocess/src/main/resources/log4j2.properties
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/subprocess/subprocess_server-test.cc
5 files changed, 113 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/4
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/10/java/kudu-subprocess/build.gradle
File java/kudu-subprocess/build.gradle:
http://gerrit.cloudera.org:8080/#/c/15628/10/java/kudu-subprocess/build.gradle@36
PS10, Line 36: compile libs.log4jCore
Do we still need log4jCore?
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 16:51:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#11).
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the properties file already exists,
the existing file is honored. This means a file will not be regenerated
if one has already been generated -- I will add a follow up patch to
allow users to overwrite the file.
This properties file will direct logs to the directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and adjusting log42j log-level to be
useful, so this also adds --ranger_logtostdout and --ranger_log_level
for these respectively. I opted not to use the glog --minloglevel flag
since they don't quite map to log4j2 log levels (e.g. glog doesn't have
debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-ranger-subprocess-log4j2.properties
kudu-ranger-subprocess.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/CMakeLists.txt
A src/kudu/subprocess/subprocess_proxy.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
9 files changed, 374 insertions(+), 65 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/11
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 11
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#8).
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the properties file already exists,
the existing file is honored. This means a file will not be regenerated
if on has already been generated -- I will add a follow up patch to
allow users to overwrite the file.
This properties file will direct logs to directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and adjusting log42j log-level to be
useful, so this also adds --ranger_logtostdout and --ranger_log_level
for these respectively. I opted not to use the glog --minloglevel flag
since they don't quite map to log4j2 log levels (e.g. glog doesn't have
debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-ranger-subprocess-log4j2.properties
kudu-ranger-subprocess.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/gradle/dependencies.gradle
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/CMakeLists.txt
A src/kudu/subprocess/subprocess_proxy.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
10 files changed, 339 insertions(+), 60 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/8
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: direct client logs to a log file
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: direct client logs to a log file
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 Apr 2020 07:09:36 +0000
Gerrit-HasComments: No
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#10).
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the properties file already exists,
the existing file is honored. This means a file will not be regenerated
if one has already been generated -- I will add a follow up patch to
allow users to overwrite the file.
This properties file will direct logs to the directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and adjusting log42j log-level to be
useful, so this also adds --ranger_logtostdout and --ranger_log_level
for these respectively. I opted not to use the glog --minloglevel flag
since they don't quite map to log4j2 log levels (e.g. glog doesn't have
debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-ranger-subprocess-log4j2.properties
kudu-ranger-subprocess.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/gradle/dependencies.gradle
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/CMakeLists.txt
A src/kudu/subprocess/subprocess_proxy.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
10 files changed, 376 insertions(+), 65 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/10
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: direct client logs to a log file
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: direct client logs to a log file
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java:
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java@252
PS5, Line 252: if (logToStdout && appenders.containsKey("SystemOutAppender")) {
Should this be the default if a file is not provided? If we remove the ability to use system.out for IO all together that would be safe and reduce the configuration options/flags.
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/resources/log4j2.properties
File java/kudu-subprocess/src/main/resources/log4j2.properties:
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/resources/log4j2.properties@30
PS5, Line 30: appender.rollingFile.type = RollingFile
If we allow generating the log4j conf in the master based on my review in the preceeding patch, then we could generate the rolling configuration to match the masters expectations and configuration without system variables. We could also match the configured '--max_log_size' and '--max_log_files'.
Alternatively if we keep using this template we should update it to reflect '--max_log_size' and '--max_log_files' in some way.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 Apr 2020 14:33:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 9: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296
PS8, Line 296: K(WriteStringToFileSync(
> That doesn't seem to work (it's not honored by log4j2 at least). Leaving th
Oh I misunderstood; I thought this was "our" parameter.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 00:19:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 7:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@12
PS7, Line 12: file
> the properties file? Does this mean the properties file cannot be updated o
Users can manually modify the file and the changes will take effect. But updating flags and restarting won't update the file unless the conf file is deleted. I can add a --ranger_overwrite_logging_config or something in a follow up patch.
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@21
PS7, Line 21: ranger_enable_debug_logging
> Would it make sense to do `--ranger_log_level` and accept standard log4j le
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@77
PS7, Line 77: client
> nit: should we remove `client`. This can contain subprocess logs too right?
I'll replace this with "subprocess".
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@81
PS7, Line 81: TAG_FLAG(ranger_logging_config_dir, advanced);
> Should we mark this as evolving?
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@85
PS7, Line 85: TAG_FLAG(ranger_enable_debug_logging, advanced);
> Should we mark this as evolving?
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@89
PS7, Line 89: TAG_FLAG(ranger_logtostdout, hidden);
> Should we also mark this as `unsafe` since it's for testing only?
There's nothing unsafe about it per se. I'll just make this advanced.
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h
File src/kudu/subprocess/subprocess_proxy.h:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@38
PS7, Line 38: namespace {
> warning: do not use unnamed namespaces in header files [google-build-namesp
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@50
PS7, Line 50: static const char* kLog4j2RollingPropertiesTemplate = R"(
> warning: variable 'kLog4j2RollingPropertiesTemplate' defined in a header fi
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@66
PS7, Line 66: static const char* kLog4j2ConsoleProperties = R"(
> warning: 'kLog4j2ConsoleProperties' is a static definition in anonymous nam
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@79
PS7, Line 79: static const char* kLog4j2PropertiesTemplate = R"(
> warning: 'kLog4j2PropertiesTemplate' is a static definition in anonymous na
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@92
PS7, Line 92: string
> nit: remove
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@122
PS7, Line 122: appender_ref_name_specs.emplace_back(
> warning: 'emplace_back' is called inside a loop; consider pre-allocating th
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h
File src/kudu/util/env.h:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h@694
PS7, Line 694: WriteStringToFileSync
> Can you comment on the difference between the above method?
Done
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 19:28:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@13
PS8, Line 13: This means a file will not be regenerated
: if on has already been generated
One downside to this approach is that we can't automatically update log4j2.properties in subsequent releases.
I think that's actually more important than providing a mechanism for admins to supply their own log4j2.properties.
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@14
PS8, Line 14: on
one
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@17
PS8, Line 17: directory
Nit: the directory
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@255
PS8, Line 255: if (!env->FileExists(log_conf_dir)) {
: RETURN_NOT_OK(env->CreateDir(log_conf_dir));
: }
I don't think we should be creating FLAGS_log_dir. Maybe condition this on whether we're using FLAGS_ranger_log_config_dir? Or even then, force admins to create it?
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296
PS8, Line 296: -Dlog4j2.configurationFile
Nit: maybe -Dlog4j2.configuration.filename?
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/subprocess/subprocess_proxy.cc
File src/kudu/subprocess/subprocess_proxy.cc:
PS8:
We're doing more and more of this "template composition", and it's getting increasingly complicated. At some point we're better off using a real templating system, which should be able to handle composition based on a variety of conditions (loops, if/else, etc.).
We've got mustache in the codebase and I think it'd be a reasonable fit. Currently mustache expects template files to live on disk, but there's no reason we can't modify it to reference them from memory.
Doesn't apply to this patch, but something to think about.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 21:30:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the properties file already exists,
the existing file is honored. This means a file will not be regenerated
if one has already been generated -- I will add a follow up patch to
allow users to overwrite the file.
This properties file will direct logs to the directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and adjusting log42j log-level to be
useful, so this also adds --ranger_logtostdout and --ranger_log_level
for these respectively. I opted not to use the glog --minloglevel flag
since they don't quite map to log4j2 log levels (e.g. glog doesn't have
debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-ranger-subprocess-log4j2.properties
kudu-ranger-subprocess.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
Reviewed-on: http://gerrit.cloudera.org:8080/15628
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/CMakeLists.txt
A src/kudu/subprocess/subprocess_proxy.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
9 files changed, 374 insertions(+), 65 deletions(-)
Approvals:
Grant Henke: Looks good to me, approved
Andrew Wong: Verified
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 12
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java:
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java@252
PS5, Line 252:
> Should this be the default if a file is not provided? If we remove the abil
Yeah, I'm going to punt on this for now; the new patch doesn't touch this code at all.
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/resources/log4j2.properties
File java/kudu-subprocess/src/main/resources/log4j2.properties:
http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/resources/log4j2.properties@30
PS5, Line 30:
> If we allow generating the log4j conf in the master based on my review in t
Done
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 03 Apr 2020 08:10:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 05:23:10 +0000
Gerrit-HasComments: No
[kudu-CR] ranger: direct client logs to a log file
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#3).
Change subject: ranger: direct client logs to a log file
......................................................................
ranger: direct client logs to a log file
This patch enables us to direct Java subprocess logs to a rolling file
with the new "-r" option. The subprocess will use the KUDU_LOG_DIR and
KUDU_LOG_NAME system properties to output logs as appropriate.
Since, in production, we expect users to prefer these logs over stdout,
this also makes logging to stdout optional with the new "-s" option.
Logging to stdout is enabled in subprocess_proxy-test and
subprocess_server-test since it's useful to view the subprocess logs
in-line with test output.
Here's what's in a Ranger-configured Kudu master's log directory (I
lowered the rolling size threshold for the sake of generating a new roll
quickly):
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.WARNING -> kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200401-193145.0.59433
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-ranger-client.awong-MBP16-21621.local.20200401-193145.log.gz
kudu-ranger-client.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
M java/kudu-subprocess/src/main/resources/log4j2.properties
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/subprocess/subprocess_server-test.cc
5 files changed, 109 insertions(+), 35 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/3
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: direct client logs to a log file
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: direct client logs to a log file
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/1//COMMIT_MSG@10
PS1, Line 10: The subprocess will use the KUDU_LOG_DIR and
: KUDU_LOG_NAME system properties to output logs as appropriate.
> Could you add a sample listing of the log directory and the subprocess log
Done
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 Apr 2020 02:40:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 11
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 20:06:52 +0000
Gerrit-HasComments: No
[kudu-CR] ranger: direct client logs to a log file
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#5).
Change subject: ranger: direct client logs to a log file
......................................................................
ranger: direct client logs to a log file
This patch enables us to direct Java subprocess logs to a rolling file
with the new "-r" option. The subprocess will use the KUDU_LOG_DIR and
KUDU_LOG_NAME system properties to output logs as appropriate.
Since, in production, we expect users to prefer these logs over stdout,
this also makes logging to stdout optional with the new "-s" option.
Logging to stdout is enabled in subprocess_proxy-test and
subprocess_server-test since it's useful to view the subprocess logs
in-line with test output.
Here's what's in a Ranger-configured Kudu master's log directory (I
lowered the rolling size threshold for the sake of generating a new roll
quickly):
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.WARNING -> kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200401-193145.0.59433
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-ranger-client.awong-MBP16-21621.local.20200401-193145.log.gz
kudu-ranger-client.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
M java/kudu-subprocess/src/main/resources/log4j2.properties
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/subprocess/subprocess_server-test.cc
5 files changed, 114 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/5
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@12
PS7, Line 12: file
the properties file? Does this mean the properties file cannot be updated once created?
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h
File src/kudu/subprocess/subprocess_proxy.h:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@92
PS7, Line 92: string
nit: remove
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h
File src/kudu/util/env.h:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h@694
PS7, Line 694: WriteStringToFileSync
Can you comment on the difference between the above method?
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 19:05:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15628/10/java/kudu-subprocess/build.gradle
File java/kudu-subprocess/build.gradle:
http://gerrit.cloudera.org:8080/#/c/15628/10/java/kudu-subprocess/build.gradle@36
PS10, Line 36: compile libs.log4jCore
> Do we still need log4jCore?
Done
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 18:48:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#9).
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the properties file already exists,
the existing file is honored. This means a file will not be regenerated
if one has already been generated -- I will add a follow up patch to
allow users to overwrite the file.
This properties file will direct logs to the directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and adjusting log42j log-level to be
useful, so this also adds --ranger_logtostdout and --ranger_log_level
for these respectively. I opted not to use the glog --minloglevel flag
since they don't quite map to log4j2 log levels (e.g. glog doesn't have
debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165
kudu-ranger-subprocess-log4j2.properties
kudu-ranger-subprocess.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/gradle/dependencies.gradle
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/CMakeLists.txt
A src/kudu/subprocess/subprocess_proxy.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
10 files changed, 365 insertions(+), 60 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/9
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 00:48:19 +0000
Gerrit-HasComments: No
[kudu-CR] ranger: enable log4j2 logging to files
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#6).
Change subject: ranger: enable log4j2 logging to files
......................................................................
ranger: enable log4j2 logging to files
This patch enables Kudu to generate a log4j2 properties file and pass it
to the Ranger client. This file is created in a directory specified by
the new --ranger_logging_config_dir flag, which, by default, will direct
the properties file to --log_dir. If the file already exists, the
existing file is honored.
This properties file will direct logs to directory pointed to by
--log_dir, and will honor the existing --max_log_files and
--max_log_size flags.
I've found logging to stdout and log42j debug-level logging to be
useful, so this also adds --ranger_logtostdout and
--ranger_enable_debug_logging for these respectively. I opted not to
use the glog --minloglevel flag since they don't quite map to log4j2 log
levels (e.g. glog doesn't have debug-level logging).
I manually tested that an existing properties file that has been
modified will be honored. Here's what's in a Ranger-configured Kudu
master's log directory:
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005349.0.5236
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005609.0.5320
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005921.0.5395
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005349.5236
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005609.5320
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395
kudu-ranger-client-log4j2.properties
kudu-ranger-client.awong-MBP16-21621.local.20200403-005349.log.gz
kudu-ranger-client.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/gradle/dependencies.gradle
M java/kudu-subprocess/build.gradle
M src/kudu/ranger/mini_ranger.cc
M src/kudu/ranger/mini_ranger.h
M src/kudu/ranger/ranger_client-test.cc
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy.h
M src/kudu/util/env.h
8 files changed, 293 insertions(+), 60 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/6
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: direct client logs to a log file
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15628
to look at the new patch set (#2).
Change subject: ranger: direct client logs to a log file
......................................................................
ranger: direct client logs to a log file
This patch enables us to direct Java subprocess logs to a rolling file
with the new "-r" option. The subprocess will use the KUDU_LOG_DIR and
KUDU_LOG_NAME system properties to output logs as appropriate.
Since, in production, we expect users to prefer these logs over stdout,
this also makes logging to stdout optional with the new "-s" option.
Logging to stdout is enabled in subprocess_proxy-test and
subprocess_server-test since it's useful to view the subprocess logs
in-line with test output.
Here's what's in a Ranger-configured Kudu master's log directory (I
lowered the rolling size threshold for the sake of generating a new roll
quickly):
kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.WARNING -> kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200401-193145.0.59433
kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200401-193144.59433
kudu-master.awong-MBP16-21621.local.awong.log.WARNING.20200401-193151.59433
kudu-ranger-client.awong-MBP16-21621.local.20200401-193145.log.gz
kudu-ranger-client.awong-MBP16-21621.local.log
Change-Id: I7efa631832c219fce214304538e6ab6442062752
---
M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java
M java/kudu-subprocess/src/main/resources/log4j2.properties
M src/kudu/ranger/ranger_client.cc
M src/kudu/subprocess/subprocess_proxy-test.cc
M src/kudu/subprocess/subprocess_server-test.cc
5 files changed, 108 insertions(+), 35 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/2
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)