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

loggers #996

Merged
merged 23 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
5b42a24
multiple loggers
adrianmroz-allegro Dec 13, 2022
1d7d9ea
simplify routes parameters
adrianmroz-allegro Dec 13, 2022
6e4f26e
pass logger explicitly
adrianmroz-allegro Dec 13, 2022
9846969
use specific loggers in CLI functions
adrianmroz-allegro Dec 13, 2022
c6f05d3
fix test compile error
adrianmroz-allegro Dec 14, 2022
1d39ba9
validate logger format setting
adrianmroz-allegro Dec 14, 2022
9f66e7c
override logger format from CLI parameter
adrianmroz-allegro Dec 14, 2022
b9d429b
use logger on server start
adrianmroz-allegro Dec 14, 2022
f6eec7b
docs typo
adrianmroz-allegro Dec 14, 2022
c0b4dcd
log errors with correct level and in one event
adrianmroz-allegro Dec 14, 2022
68146f1
change signature for readability
adrianmroz-allegro Dec 14, 2022
a6b91cd
fix tests
adrianmroz-allegro Dec 14, 2022
ae617a2
reduce exports from logger.ts - logger should be passed directly, no …
adrianmroz-allegro Dec 14, 2022
495de22
logger that redirects everything to stderr
adrianmroz-allegro Dec 14, 2022
2fb53c8
push loggers further
adrianmroz-allegro Dec 14, 2022
638bc90
utils for now in iso format
adrianmroz-allegro Dec 14, 2022
a6feafe
cleaner Logger API and types
adrianmroz-allegro Dec 14, 2022
1d164a7
rename to @timestamp
adrianmroz-allegro Dec 15, 2022
fe3ecc3
add docs
adrianmroz-allegro Dec 15, 2022
88b0621
let commander.js validate logger format values
adrianmroz-allegro Dec 15, 2022
da3a4b0
util for logging js error objects
adrianmroz-allegro Dec 15, 2022
5b8f97b
refactor for brevity
adrianmroz-allegro Dec 15, 2022
d97135a
readiness error should be logged as warn
adrianmroz-allegro Dec 15, 2022
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
11 changes: 11 additions & 0 deletions docs/configuration-cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ The port that Turnilo should run on.

Indicates that Turnilo should run in verbose mode. This will log all the queries done by Turnilo.

**loggerFormat** *EXPERIMENTAL* (`plain` or `json`), default: `plain`

Format for logged message.
* `plain`: messages are logged as is.
* `json`: messages are wrapped in object with additional metadata and logged as stringified JSON.

Additional metadata for `json` format:
* `@timestamp`: ISO 8601 timestamp of logged event
* `level`: "INFO", "WARN", or "ERROR" string
* `logger`: name of the logger

**serverHost** (string), default: bind to all hosts

The host that Turnilo will bind to.
Expand Down
2 changes: 1 addition & 1 deletion docs/extending-turnilo.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ exports.druidRequestDecoratorFactory = function (logger, params) {
return function () {
return {
headers: {
"X-I-Like": auth
"X-I-Like": like
},
};
};
Expand Down
104 changes: 88 additions & 16 deletions src/common/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,41 +14,113 @@
* limitations under the License.
*/

import { LoggerFormat } from "../../server/models/server-settings/server-settings";
import { noop, Unary } from "../utils/functional/functional";
import { isNil } from "../utils/general/general";
import { isoNow } from "../utils/time/time";

type LogFn = (msg: string, extra?: Record<string, string>) => void;
type LogLevel = "INFO" | "WARN" | "ERROR";

export function errorToMessage(error: Error): string {
if (isNil(error.stack)) {
return error.message;
}
return `${error.message}\n${error.stack}`;
}

export interface Logger {
log: Function;
warn: Function;
error: Function;
addPrefix: Unary<String, Logger>;
log: LogFn;
warn: LogFn;
error: LogFn;
setLoggerId: Unary<String, Logger>;
}

class JSONLogger implements Logger {

constructor(private logger = "turnilo") {
}

private logMessage(level: LogLevel, message: string, extra: Record<string, string> = {}) {
console.log(JSON.stringify({
message,
level,
"@timestamp": isoNow(),
"logger": this.logger,
...extra
}));
}

log(message: string, extra: Record<string, string> = {}) {
this.logMessage("INFO", message, extra);
}

error(message: string, extra: Record<string, string> = {}) {
this.logMessage("ERROR", message, extra);
}

warn(message: string, extra: Record<string, string> = {}) {
this.logMessage("WARN", message, extra);
}

setLoggerId(loggerId: string): Logger {
return new JSONLogger(loggerId);
}
}

class ConsoleLogger implements Logger {
constructor(private prefixes: string[] = []) {
constructor(private prefix = "") {
}

error(...args: any[]) {
console.error(...this.prefixes, ...args);
error(message: string) {
console.error(this.prefix, message);
}

warn(...args: any[]) {
console.warn(...this.prefixes, ...args);
warn(message: string) {
console.warn(this.prefix, message);
}

log(...args: any[]) {
console.log(...this.prefixes, ...args);
log(message: string) {
console.log(this.prefix, message);
}

addPrefix(prefix: string): Logger {
return new ConsoleLogger([...this.prefixes, prefix]);
setLoggerId(loggerId: string): Logger {
return new ConsoleLogger(loggerId);
}
}

export const LOGGER: Logger = new ConsoleLogger();
class AlwaysStdErrLogger implements Logger {
setLoggerId(): Logger {
return this;
}

error(message: string) {
console.error(message);
}

log(message: string) {
console.error(message);
}

warn(message: string) {
console.error(message);
}
}

export const NULL_LOGGER: Logger = {
export const NOOP_LOGGER: Logger = {
adrianmroz-allegro marked this conversation as resolved.
Show resolved Hide resolved
error: noop,
warn: noop,
log: noop,
addPrefix: noop
setLoggerId: () => NOOP_LOGGER
};

const LOGGERS: Record<LoggerFormat, Logger> = {
noop: NOOP_LOGGER,
json: new JSONLogger(),
plain: new ConsoleLogger(),
error: new AlwaysStdErrLogger()
} as const;

export function getLogger(format: LoggerFormat): Logger {
return LOGGERS[format];
}
7 changes: 4 additions & 3 deletions src/common/models/app-settings/app-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

import { Executor } from "plywood";
import { Logger } from "../../logger/logger";
import {
ClientCustomization,
Customization,
Expand Down Expand Up @@ -62,10 +63,10 @@ export interface ClientAppSettings {
readonly oauth: Oauth;
}

export function fromConfig(config: AppSettingsJS): AppSettings {
export function fromConfig(config: AppSettingsJS, logger: Logger): AppSettings {
const clientTimeout = config.clientTimeout === undefined ? DEFAULT_CLIENT_TIMEOUT : config.clientTimeout;
const version = config.version || 0;
const customization = customizationFromConfig(config.customization);
const customization = customizationFromConfig(config.customization, logger);
const oauth = oauthFromConfig(config.oauth);

return {
Expand All @@ -76,7 +77,7 @@ export function fromConfig(config: AppSettingsJS): AppSettings {
};
}

export const EMPTY_APP_SETTINGS = fromConfig({});
export const emptySettings = (logger: Logger) => fromConfig({}, logger);

export function serialize({ oauth, clientTimeout, customization, version }: AppSettings): SerializedAppSettings {
return {
Expand Down
7 changes: 4 additions & 3 deletions src/common/models/cluster/cluster.fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import { NOOP_LOGGER } from "../../logger/logger";
import { Cluster, ClusterJS, fromConfig } from "./cluster";

export class ClusterFixtures {
Expand All @@ -33,7 +34,7 @@ export class ClusterFixtures {
}

static druidWikiCluster(): Cluster {
return fromConfig(ClusterFixtures.druidWikiClusterJS());
return fromConfig(ClusterFixtures.druidWikiClusterJS(), NOOP_LOGGER);
}

static druidTwitterClusterJS(): ClusterJS {
Expand All @@ -52,7 +53,7 @@ export class ClusterFixtures {
}

static druidTwitterCluster(): Cluster {
return fromConfig(ClusterFixtures.druidTwitterClusterJS());
return fromConfig(ClusterFixtures.druidTwitterClusterJS(), NOOP_LOGGER);
}

static druidTwitterClusterJSWithGuard(guardDataCubes = true): Cluster {
Expand All @@ -68,6 +69,6 @@ export class ClusterFixtures {
guardDataCubes,

introspectionStrategy: "segment-metadata-fallback"
});
}, NOOP_LOGGER);
}
}
19 changes: 11 additions & 8 deletions src/common/models/cluster/cluster.mocha.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { expect, use } from "chai";
import equivalent from "../../../client/utils/test-utils/equivalent";
import { RequestDecorator } from "../../../server/utils/request-decorator/request-decorator";
import { RetryOptions } from "../../../server/utils/retry-options/retry-options";
import { NOOP_LOGGER } from "../../logger/logger";
import { ClusterAuthJS } from "../cluster-auth/cluster-auth";
import { Cluster, ClusterJS, fromConfig } from "./cluster";

Expand All @@ -31,15 +32,15 @@ function buildCluster(options: Partial<ClusterJS> = {}): Cluster {
return fromConfig({
...baseConfig,
...options
});
}, NOOP_LOGGER);
}

use(equivalent);

describe("Cluster", () => {
describe("fromConfig", () => {
it("should load default values", () => {
const cluster = fromConfig(baseConfig);
const cluster = buildCluster();

expect(cluster).to.be.deep.equal({
name: "foobar",
Expand All @@ -64,19 +65,19 @@ describe("Cluster", () => {
});

it("should throw with incorrect name type", () => {
expect(() => fromConfig({ ...baseConfig, name: 1 } as unknown as ClusterJS)).to.throw("must be a string");
expect(() => buildCluster({ name: 1 } as unknown as ClusterJS)).to.throw("must be a string");
});

it("should throw with incorrect empty name", () => {
expect(() => fromConfig({ ...baseConfig, name: "" })).to.throw("empty name");
expect(() => buildCluster({ name: "" })).to.throw("empty name");
});

it("should throw with not url safe name", () => {
expect(() => fromConfig({ ...baseConfig, name: "foobar%bazz#" })).to.throw("is not a URL safe name");
expect(() => buildCluster({ name: "foobar%bazz#" })).to.throw("is not a URL safe name");
});

it("should throw with name equal to native", () => {
expect(() => fromConfig({ ...baseConfig, name: "native" })).to.throw("name can not be 'native'");
expect(() => buildCluster({ name: "native" })).to.throw("name can not be 'native'");
});

it("should read auth options", () => {
Expand Down Expand Up @@ -119,7 +120,7 @@ describe("Cluster", () => {
});

it("should read request decorator", () => {
const cluster = fromConfig({
const cluster = buildCluster({
name: "foobar",
url: "http://foobar",
requestDecorator: {
Expand All @@ -141,8 +142,10 @@ describe("Cluster", () => {
});

it("should read old host and assume http protocol", () => {
const cluster = fromConfig({
const cluster = buildCluster({
name: "old-host",
// Set url to undefined, to force old format handling
url: undefined,
host: "broker-host.com"
} as unknown as ClusterJS);

Expand Down
9 changes: 5 additions & 4 deletions src/common/models/cluster/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { External } from "plywood";
import { URL } from "url";
import { RequestDecorator, RequestDecoratorJS } from "../../../server/utils/request-decorator/request-decorator";
import { RetryOptions, RetryOptionsJS } from "../../../server/utils/retry-options/retry-options";
import { Logger } from "../../logger/logger";
import { isNil, isTruthy, optionalEnsureOneOf, verifyUrlSafeName } from "../../utils/general/general";
import { ClusterAuth, ClusterAuthJS, readClusterAuth } from "../cluster-auth/cluster-auth";

Expand Down Expand Up @@ -117,9 +118,9 @@ function readUrl(cluster: any): string {
throw new Error("Cluster: missing url field");
}

function readRequestDecorator(cluster: any): RequestDecorator | null {
function readRequestDecorator(cluster: any, logger: Logger): RequestDecorator | null {
if (typeof cluster.requestDecorator === "string" || !isNil(cluster.decoratorOptions)) {
console.warn(`Cluster ${cluster.name} : requestDecorator as string and decoratorOptions fields are deprecated. Use object with path and options fields`);
logger.warn(`Cluster ${cluster.name} : requestDecorator as string and decoratorOptions fields are deprecated. Use object with path and options fields`);
return RequestDecorator.fromJS({ path: cluster.requestDecorator, options: cluster.decoratorOptions });
}
if (isTruthy(cluster.requestDecorator)) return RequestDecorator.fromJS(cluster.requestDecorator);
Expand All @@ -145,7 +146,7 @@ function readInterval(value: number | string, defaultValue: number): number {
return numberValue;
}

export function fromConfig(params: ClusterJS): Cluster {
export function fromConfig(params: ClusterJS, logger: Logger): Cluster {
const {
name,
sourceListScan = DEFAULT_SOURCE_LIST_SCAN,
Expand All @@ -167,7 +168,7 @@ export function fromConfig(params: ClusterJS): Cluster {
const sourceListRefreshInterval = readInterval(params.sourceListRefreshInterval, DEFAULT_SOURCE_LIST_REFRESH_INTERVAL);
const sourceTimeBoundaryRefreshInterval = readInterval(params.sourceTimeBoundaryRefreshInterval, DEFAULT_SOURCE_TIME_BOUNDARY_REFRESH_INTERVAL);
const retry = RetryOptions.fromJS(params.retry);
const requestDecorator = readRequestDecorator(params);
const requestDecorator = readRequestDecorator(params, logger);
const auth = readClusterAuth(params.auth);

const url = readUrl(params);
Expand Down
Loading