zowelog zss format handler #160
Conversation
Signed-off-by: sakeerthy <[email protected]>
Signed-off-by: sakeerthy <[email protected]>
Signed-off-by: sakeerthy <[email protected]>
Signed-off-by: sakeerthy <[email protected]>
Signed-off-by: sakeerthy <[email protected]>
Signed-off-by: sakeerthy <[email protected]>
| unsigned int id = compID & 0xFFFFFFFF; | ||
| if(compID >= LOG_PROD_COMMON && compID < LOG_PROD_ZIS) { | ||
| snprintf(prefix,sizeof(LOG_PREFIX_ZCC),LOG_PREFIX_ZCC); | ||
| switch (id) { // most of these don't actally log, but are defined in logging.h |
There was a problem hiding this comment.
Can you please use the switch statement format we've already used in the ZSS code?
For example,
static int translateLogLevelToClientTraceLevel(int logLevel) {
int traceLevel = CLIENT_TRACE_LEVEL_NA;
switch (logLevel) {
case ZOWE_LOG_SEVERE:
traceLevel = CLIENT_TRACE_LEVEL_SEVERE;
break;
case ZOWE_LOG_WARNING:
traceLevel = CLIENT_TRACE_LEVEL_WARNING;
break;
case ZOWE_LOG_INFO:
traceLevel = CLIENT_TRACE_LEVEL_INFO;
break;
case ZOWE_LOG_DEBUG:
traceLevel = CLIENT_TRACE_LEVEL_FINE;
break;
case ZOWE_LOG_DEBUG2:
traceLevel = CLIENT_TRACE_LEVEL_FINER;
break;
case ZOWE_LOG_DEBUG3:
traceLevel = CLIENT_TRACE_LEVEL_FINEST;
break;
}
return traceLevel;
}| ACEE *acee; | ||
| acee = getAddressSpaceAcee(); | ||
| char user[USER_SIZE] = { 0 }; | ||
| snprintf(user,sizeof(user),acee->aceeuser+1); |
There was a problem hiding this comment.
Consider putting spaces between argument, it's much easier to read it that way.
|
|
||
| stck += getLocalTimeOffset(); | ||
|
|
||
| stckToLogTimestamp(stck, timestamp); |
There was a problem hiding this comment.
Is there anything better in LE? I think you can get pretty formatted timestamp using the standard library.
There was a problem hiding this comment.
It's easy to create an ISO8601 timestamp using C POSIX functions.
struct timeval tv;
struct tm tm;
gettimeofday(&tv, NULL);
localtime_r(&tv.tv_sec, &tm);
static const size_t ISO8601_LEN = 30;
char datetime[ISO8601_LEN];
char timestamp[ISO8601_LEN]; // ISO-8601 timestamp
strftime(datetime, sizeof datetime, "%Y-%m-%dT%H:%M:%S", &tm);
snprintf(timestamp, sizeof timestamp, "%s.%d", datetime, tv.tv_usec)
puts(timestamp); |
|
||
| unsigned int id = compID & 0xFFFFFFFF; | ||
| if(compID >= LOG_PROD_COMMON && compID < LOG_PROD_ZIS) { | ||
| snprintf(prefix,sizeof(LOG_PREFIX_ZCC),LOG_PREFIX_ZCC); |
There was a problem hiding this comment.
Since you don't need to do complex formatting, consider using something more lightweight than snprintf.
| } | ||
| else if (compID > LOG_PROD_PLUGINS) { | ||
| snprintf(suffix,sizeof(component->name),"%s",(strrchr(component->name, '/')+1)); // given more characters than it writes | ||
| snprintf(prefix,sizeof(component->name)-sizeof(suffix),"%s",component->name); |
There was a problem hiding this comment.
sizeof(component->name) is either 4 or 8, sizeof(suffix) is 128, you'll get either 4294967172 or 4294967176.
| } | ||
|
|
||
| char *filename; | ||
| filename = strrchr(path, '/'); // returns a pointer to the last occurence of '/' |
There was a problem hiding this comment.
What if there is no /?
| filename+=1; | ||
| // formatting + prefix + suffix + filename + line number | ||
| int locationSize = LOCATION_PREFIX_PADDING + strlen(prefix) + strlen(suffix) + strlen(filename) + LOCATION_SUFFIX_PADDING; | ||
| *locationInfo = (char*) safeMalloc(locationSize,"locationInfo"); |
There was a problem hiding this comment.
I think it's a really bad idea to allocate storage in a logging function. Even so, where is the NULL check? Where do you free this storage?
| break; | ||
| case LOG_COMP_CMS: snprintf(suffix,sizeof(LOG_COMP_TEXT_CMS),LOG_COMP_TEXT_CMS); | ||
| break; | ||
| case 0xC0001: snprintf(suffix,sizeof(LOG_COMP_TEXT_CMS),LOG_COMP_TEXT_CMS); |
| } | ||
| } | ||
| else if (compID > LOG_PROD_PLUGINS) { | ||
| snprintf(suffix,sizeof(component->name),"%s",(strrchr(component->name, '/')+1)); // given more characters than it writes |
There was a problem hiding this comment.
What if strrchr is NULL.
Signed-off-by: sakeerthy <[email protected]>
Dependent on zowe/zowe-common-c#126
Do not merge this until zowe common c is approved/merged so this can point to the correct zowe common c
Signed-off-by: sakeerthy [email protected]