Yeah, that syntax is better although it doesn’t solve the multiple appender refs case either.
LoggerConfig accepts a List<AppenderRef> so it would seem that if we modify LoggerConfig to add another attribute that is a string that creates an AppenderRef for each item then one could do <Logger name=“xyzzy” level=“INFO” appenderRefNames=“a, b, c”/> Ralph > On Jan 10, 2022, at 5:04 AM, Volkan Yazıcı <[email protected]> wrote: > > I am curious if shall we adopt a more generic approach to extraction: > > <CompoundProperty> > <Source>${sys:hadoop.root.logger}</Source> > <Targets pattern="^(.+)\s*,\s(.+)$"> > <Target>hadoop.root.logger.level</Target> > <Target>hadoop.root.logger.appender</Target> > </Targets> > </CompoundProperty> > > On Mon, Jan 10, 2022 at 5:53 AM Ralph Goers <[email protected]> > wrote: > >> OK, I had a suspicion the reason for needing this was something like that. >> >> The syntax you propose below would require modifying PropertiesUtil, the >> Property >> plugin and StrSubstitutor to support arrays. I don’t think I’d want to go >> down that path. >> StrSubstitutor is already complicated and the idea of enhancing it to >> support stuff like >> this doesn’t excite me. >> >> I think we’d be better off creating a new plugin for this. Something like: >> >> <CompoundProperty name=“hadoop.root.logger.level” split=“,” >> index=“0”>${sys:hadoop.root.logger}</CompoundProperty> >> <CompoundProperty name=“hadoop.root.logger.appender” split=“,” >> index=“1”>${sys:hadoop.root.logger}</CompoundProperty> >> >> The only problem here is if you want to provide more than one appender >> this won’t >> work well. But I am sure we could enhance AppenderRef to accept a list of >> appenders. >> >> Ralph >> >> >> >> >>> On Jan 9, 2022, at 5:29 PM, 张铎(Duo Zhang) <[email protected]> wrote: >>> >>> Thank you for the reply. >>> >>> And yes, it is not only for the root logger, we have tons of different >>> loggers in hadoop, as hadoop is constructed with several different >> projects >>> actually, and they are developed by different groups of people... >>> >>> And on splitting the config in shell, actually this is exactly what I >> have >>> done in HBase >>> >>> See >>> >> https://github.com/apache/hbase/blob/3ec3df5887e9271f7e75779eafe2439012cfb2c3/bin/hbase#L829 >>> >>> And also >>> >> https://github.com/apache/hbase/blob/3ec3df5887e9271f7e75779eafe2439012cfb2c3/bin/hbase.cmd#L336 >>> >>> For HBase maybe it is acceptable but hadoop is another story. >>> Hadoop is widely used almost in every bigdata platform. Among the >>> companies I've worked with, they all have their own hadoop deployment >>> systems which have modified version of start up scripts... >>> >>> I can imagine that if we do the same trick in HBase, then people will ask >>> again and again on the mailing list why passing -Dhadoop.root.logger does >>> not work, as well as other options like -Dyarn.root.logger, >>> -Dhadoop.security.logger, etc... >>> >>> So it will be good if we can support configure level and appenders in one >>> property. >>> >>> Or maybe another way is to enhance the ability in the Properties section, >>> so we can split one property into two properties, something like >>> >>> <Properties> >>> <Property name="hadoop.root.logger" >>> type="array">${sys:hadoop.root.logger:-INFO,Console}</Property> >>> <Property >>> name="hadoop.root.logger.level">${hadoop.root.logger[0]}</Property> >>> <Property name="hadoop.root.logger.appender"> ${hadoop.root.logger[1]} >>> </Property> >>> <Properties> >>> >>> Notice that since we could add multiple appenders here, maybe we need to >>> support something like ${hadoop.root.logger[1:] to combine all the >>> appenders... >>> This could also solve the problem as people could still pass >>> -Dhadoop.root.logger. >>> >>> But I'm not sure if adding the ability to split a string in configuation >>> could introduce new possible security concerns... >>> >>> Thanks. >>> >>> Ralph Goers <[email protected]> 于2022年1月10日周一 07:14写道： >>> >>>> We already support this with two variables. But if they only want to >> pass >>>> one then we have two options: >>>> 1. Modify PropertiesConfiguration to support a new element that allows a >>>> Level and AppenderRef which then gets split internally. >>>> 2. Create a Lookup that extracts the relevant portion of the data. >>>> >>>> Ralph >>>> >>>>> On Jan 9, 2022, at 3:08 PM, Gary Gregory <[email protected]> >> wrote: >>>>> >>>>> I think it is reasonable to say we can support this through 2 instead >> of >>>> 1 >>>>> variable. >>>>> >>>>> Duo? >>>>> >>>>> Gary >>>>> >>>>> On Sun, Jan 9, 2022, 16:24 Ralph Goers <[email protected]> >>>> wrote: >>>>> >>>>>> I’m looking at this and have a couple of concerns. The script has >>>>>> >>>>>> >>>>>> HADOOP_ROOT_LOGGER=${HADOOP_ROOT_LOGGER:-${HADOOP_LOGLEVEL},console} >>>>>> >>>>>> >>>> >> HADOOP_DAEMON_ROOT_LOGGER=${HADOOP_DAEMON_ROOT_LOGGER:-${HADOOP_LOGLEVEL},RFA} >>>>>> HADOOP_SECURITY_LOGGER=${HADOOP_SECURITY_LOGGER:-INFO,NullAppender} >>>>>> >>>>>> So it seems you need this for more than just the root logger. >>>>>> >>>>>> Second, you are asking us to accept “level, appender” as the value >> and >>>>>> under the covers split the >>>>>> value and assign the level to the level attribute and the appender >> name >>>> to >>>>>> the AppenderRef. >>>>>> >>>>>> While this certainly can be done it seems like it would be just as >> easy >>>> to >>>>>> do the split in the script >>>>>> and create two different variables for the logging configuration to >> pick >>>>>> up. Is there a reason that >>>>>> you can’t do this - for example perhaps users have hacked your scripts >>>> and >>>>>> now you can’t >>>>>> change them without breaking them? If so, was this “supported”? >>>>>> >>>>>> Ralph >>>>>> >>>>>> >>>>>> >>>>>>> On Jan 9, 2022, at 8:01 AM, 张铎(Duo Zhang) <[email protected]> >>>> wrote: >>>>>>> >>>>>>> I brought this up in the incubator mailing list, and was suggested to >>>>>>> report directly to the log4j community. >>>>>>> >>>>>>> https://issues.apache.org/jira/browse/HADOOP-16206 >>>>>>> >>>>>>> In hadoop we started to try migrating to log4j2 long ago, but it is >> not >>>>>>> easy. For now, one of the most blocker issues is the lack of support >> of >>>>>>> 'log4j.rootLogger=INFO,Console' grammar. Notice that we do not want >> to >>>>>> stay >>>>>>> on the bridge api, we want to fully migrate to log4j2 finally, so >>>>>>> supporting the above grammar in log4j1 bridge is not enough. >>>>>>> >>>>>>> This is the main log4j configuration file for hadoop >>>>>>> >>>>>>> >>>>>> >>>> >> https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/conf/log4j.properties >>>>>>> >>>>>>> We have this in the file >>>>>>> >>>>>>> # Define the root logger to the system property "hadoop.root.logger". >>>>>>>> log4j.rootLogger=${hadoop.root.logger} >>>>>>> >>>>>>> >>>>>>> So our users could simply pass -Dhadoop.root.logger to control the >>>> level >>>>>>> and appender. >>>>>>> >>>>>>> >>>>>> >>>> >> https://github.com/apache/hadoop/blob/39efbc6b6fe53abed15f5639edcbaaa2a9dda6d2/hadoop-common-project/hadoop-common/src/main/bin/hadoop-functions.sh#L904 >>>>>>> >>>>>>> Here we use an environment variable in our start up scripts. And I >>>>>> believe >>>>>>> there are lots of other hadoop deployment systems which will have >> their >>>>>> own >>>>>>> start scripts. >>>>>>> >>>>>>> So in general, it is just impossible for us to drop the >>>>>>> -Dhadoop.root.logger way of configuring our logging system. >>>>>>> >>>>>>> So here I want to ask if it is possible for log4j2 to still support >> the >>>>>>> 'log4j.rootLogger=INFO,Console' grammar. I'm not saying you must add >> it >>>>>>> back with no change, we just need a way to configure the level and >>>>>> appeners >>>>>>> at once, so something like >>>>>>> 'log4j2.rootLogger.levelAndAppender=INFO,Console' is also OK. >>>>>>> >>>>>>> Thanks. >>>>>> >>>>>> >>>> >>>> >> >>
