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

new-log-viewer: Define interface for filtering by log level and add support (internally) to JsonlDecoder; Move timestamp field extraction from LogbackFormatter to JsonlDecoder. #79

Merged
merged 25 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions new-log-viewer/src/services/LogFileManager/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
Decoder,
DecoderOptionsType,
LOG_EVENT_FILE_END_IDX,
} from "../../typings/decoders";
import {MAX_V8_STRING_LENGTH} from "../../typings/js";
import {
Expand Down Expand Up @@ -52,10 +51,10 @@ class LogFileManager {
this.#pageSize = pageSize;
this.#decoder = decoder;

// Build index for the entire file
const buildIdxResult = decoder.buildIdx(0, LOG_EVENT_FILE_END_IDX);
if (null !== buildIdxResult && 0 < buildIdxResult.numInvalidEvents) {
console.error("Invalid events found in decoder.buildIdx():", buildIdxResult);
// Build index for the entire file.
const buildResult = decoder.build();
if (0 < buildResult.numInvalidEvents) {
console.error("Invalid events found in decoder.build():", buildResult);
Comment on lines +54 to +57
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider enhancing error handling for invalid events

While logging invalid events using console.error aids in debugging, it might be beneficial to implement a more robust error-handling mechanism. This could include counting invalid events, aggregating them for reporting, or providing feedback to the user if a significant number are encountered.

}

this.#numEvents = decoder.getEstimatedNumEvents();
Expand Down Expand Up @@ -122,13 +121,11 @@ class LogFileManager {
return decoder;
}

/**
* Sets options for the decoder.
*
/* Sets any formatter options that exist in the decoder's options.
* @param options
*/
setDecoderOptions (options: DecoderOptionsType) {
this.#decoder.setDecoderOptions(options);
setFormatterOptions (options: DecoderOptionsType) {
this.#decoder.setFormatterOptions(options);
}

/**
Expand All @@ -144,9 +141,10 @@ class LogFileManager {
logs: string,
} {
const endLogEventIdx = Math.min(beginLogEventIdx + EXPORT_LOGS_CHUNK_SIZE, this.#numEvents);
const results = this.#decoder.decode(
const results = this.#decoder.decodeRange(
beginLogEventIdx,
endLogEventIdx
endLogEventIdx,
false,
);

if (null === results) {
Expand Down Expand Up @@ -185,7 +183,12 @@ class LogFileManager {
matchingLogEventNum,
} = this.#getCursorData(cursor);

const results = this.#decoder.decode(pageBeginLogEventNum - 1, pageEndLogEventNum - 1);
const results = this.#decoder.decodeRange(
pageBeginLogEventNum - 1,
pageEndLogEventNum - 1,
false
);

if (null === results) {
throw new Error("Error occurred during decoding. " +
`pageBeginLogEventNum=${pageBeginLogEventNum}, ` +
Expand Down
4 changes: 2 additions & 2 deletions new-log-viewer/src/services/MainWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ onmessage = async (ev: MessageEvent<MainWorkerReqMessage>) => {
throw new Error("Log file manager hasn't been initialized");
}
if ("undefined" !== typeof args.decoderOptions) {
LOG_FILE_MANAGER.setDecoderOptions(args.decoderOptions);
LOG_FILE_MANAGER.setFormatterOptions(args.decoderOptions);
}

let decodedEventIdx = 0;
Expand Down Expand Up @@ -85,7 +85,7 @@ onmessage = async (ev: MessageEvent<MainWorkerReqMessage>) => {
throw new Error("Log file manager hasn't been initialized");
}
if ("undefined" !== typeof args.decoderOptions) {
LOG_FILE_MANAGER.setDecoderOptions(args.decoderOptions);
LOG_FILE_MANAGER.setFormatterOptions(args.decoderOptions);
}
postResp(
WORKER_RESP_CODE.PAGE_DATA,
Expand Down
34 changes: 30 additions & 4 deletions new-log-viewer/src/services/decoders/ClpIrDecoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import {Nullable} from "../../typings/common";
import {
Decoder,
DecodeResultType,
FilteredLogEventMap,
LOG_EVENT_FILE_END_IDX,
LogEventCount,
} from "../../typings/decoders";
import {LogLevelFilter} from "../../typings/logs";


class ClpIrDecoder implements Decoder {
Expand All @@ -31,19 +34,42 @@ class ClpIrDecoder implements Decoder {
return this.#streamReader.getNumEventsBuffered();
}

buildIdx (beginIdx: number, endIdx: number): Nullable<LogEventCount> {
// eslint-disable-next-line class-methods-use-this
getFilteredLogEventMap (): FilteredLogEventMap {
// eslint-disable-next-line no-warning-comments
// TODO: Update this after log level filtering is implemented in clp-ffi-js
console.error("Not implemented.");

return null;
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better if we make the log-level filtering a no-op rather than completely disabling the decoder?

Suggested change
return null;
console.error("Not implemented.");
return Array.from({length: this.#streamReader.getNumEventsBuffered()}, (_, i) => i);

Copy link
Contributor Author

@davemarco davemarco Oct 1, 2024

Choose a reason for hiding this comment

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

I put in the console error but not the return value. If the value is null, the new plan is it will just not use the filter, so it is okay to return null.

}

// eslint-disable-next-line @typescript-eslint/no-unused-vars, class-methods-use-this
setLogLevelFilter (logLevelFilter: LogLevelFilter): boolean {
// eslint-disable-next-line no-warning-comments
// TODO: Update this after log level filtering is implemented in clp-ffi-js
console.error("Not implemented.");

return false;
davemarco marked this conversation as resolved.
Show resolved Hide resolved
}

build (): LogEventCount {
return {
numInvalidEvents: 0,
numValidEvents: this.#streamReader.deserializeRange(beginIdx, endIdx),
numValidEvents: this.#streamReader.deserializeRange(0, LOG_EVENT_FILE_END_IDX),
};
}

// eslint-disable-next-line class-methods-use-this
setDecoderOptions (): boolean {
setFormatterOptions (): boolean {
return true;
}

decode (beginIdx: number, endIdx: number): Nullable<DecodeResultType[]> {
decodeRange (
beginIdx: number,
endIdx: number,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
useFilter: boolean
davemarco marked this conversation as resolved.
Show resolved Hide resolved
): Nullable<DecodeResultType[]> {
return this.#streamReader.decodeRange(beginIdx, endIdx);
}
}
Expand Down
186 changes: 0 additions & 186 deletions new-log-viewer/src/services/decoders/JsonlDecoder.ts

This file was deleted.

Loading
Loading