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

Pattern format and date docs #1012

Closed
thernstig opened this issue May 15, 2020 · 3 comments · Fixed by #1163
Closed

Pattern format and date docs #1012

thernstig opened this issue May 15, 2020 · 3 comments · Fixed by #1163
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@thernstig
Copy link

According to https://github.com/log4js-node/log4js-node/blob/master/docs/layouts.md#pattern-format the pattern options are:

%d date, formatted - default is ISO8601, format options are: ISO8601, ISO8601_WITH_TZ_OFFSET, ABSOLUTE, DATE, or any string compatible with the date-format library. e.g. %d{DATE}, %d{yyyy/MM/dd-hh.mm.ss}

These names do not correspond to what is seen in the link specified, see https://www.npmjs.com/package/date-format#formatting-dates-as-strings

I assume this could be because log4js uses an older version? In any sense, it is confusing for users as one cannot check the npm site for the options, as they aren't completely identical.

@lamweili lamweili added this to the 6.4.1 milestone Jan 20, 2022
@lamweili lamweili added the documentation Improvements or additions to documentation label Jan 20, 2022
@lamweili
Copy link
Contributor

lamweili commented Jan 20, 2022

Perhaps this isn't clear, but there is a mapping from reserved log4js Strings in pattern options to date-format library options.

Reserved log4js String dateFormat Constant (and String equivalent) Output (local time)
"ISO8601" dateFormat.ISO8601_FORMAT
"yyyy-MM-ddThh:mm:ss.SSS"
2017-03-14T14:10:20.391
"ISO8601_WITH_TZ_OFFSET" dateFormat.ISO8601_WITH_TZ_OFFSET_FORMAT
"yyyy-MM-ddThh:mm:ss.SSSO"
2017-03-14T14:10:20.391+11:00
"ABSOLUTE" dateFormat.ABSOLUTETIME_FORMAT
"hh:mm:ss.SSS"
14:10:20.391
"DATE" dateFormat.DATETIME_FORMAT
"dd MM yyyy hh:mm:ss.SSS"
14 03 2017 14:10:20.391

The reserved Strings must match wholesomely.

log4js-node/lib/layouts.js

Lines 142 to 159 in 9fdbed5

function formatAsDate(loggingEvent, specifier) {
let format = dateFormat.ISO8601_FORMAT;
if (specifier) {
format = specifier;
// Pick up special cases
if (format === 'ISO8601') {
format = dateFormat.ISO8601_FORMAT;
} else if (format === 'ISO8601_WITH_TZ_OFFSET') {
format = dateFormat.ISO8601_WITH_TZ_OFFSET_FORMAT;
} else if (format === 'ABSOLUTE') {
format = dateFormat.ABSOLUTETIME_FORMAT;
} else if (format === 'DATE') {
format = dateFormat.DATETIME_FORMAT;
}
}
// Format the date
return dateFormat.asString(format, loggingEvent.startTime);
}

Perhaps the documentations need to be clearer.

@thernstig
Copy link
Author

My intention was to check that if maybe ABSOLUTE should be renamed to ABSOLUTETIME and DATE to DATETIME to correspond 1-to-1 to what is in date-format. It is a minor thing really and possibly not worth it, since it is a breaking change. Or you could allow both e.g. ABSOLUTE and ABSOLUTETIME to map to dateFormat.ABSOLUTETIME_FORMAT. In any case it is not a big thing and maybe does not make sense, so it's possible this can be closed.

@lamweili
Copy link
Contributor

lamweili commented Jan 21, 2022

That's a good suggestion to have both aliases.
One could be marked as deprecated (so it's non-breaking) for future releases in view of consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
2 participants