diff --git a/docs/configuration-cluster.md b/docs/configuration-cluster.md index 96b8d57c5..ed3c6ec1a 100644 --- a/docs/configuration-cluster.md +++ b/docs/configuration-cluster.md @@ -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. diff --git a/docs/extending-turnilo.md b/docs/extending-turnilo.md index 7a1ee2dc5..a5fb49e8f 100644 --- a/docs/extending-turnilo.md +++ b/docs/extending-turnilo.md @@ -53,7 +53,7 @@ exports.druidRequestDecoratorFactory = function (logger, params) { return function () { return { headers: { - "X-I-Like": auth + "X-I-Like": like }, }; }; diff --git a/src/common/logger/logger.ts b/src/common/logger/logger.ts index 0b49ef125..08639c966 100644 --- a/src/common/logger/logger.ts +++ b/src/common/logger/logger.ts @@ -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) => 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; + log: LogFn; + warn: LogFn; + error: LogFn; + setLoggerId: Unary; +} + +class JSONLogger implements Logger { + + constructor(private logger = "turnilo") { + } + + private logMessage(level: LogLevel, message: string, extra: Record = {}) { + console.log(JSON.stringify({ + message, + level, + "@timestamp": isoNow(), + "logger": this.logger, + ...extra + })); + } + + log(message: string, extra: Record = {}) { + this.logMessage("INFO", message, extra); + } + + error(message: string, extra: Record = {}) { + this.logMessage("ERROR", message, extra); + } + + warn(message: string, extra: Record = {}) { + 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 = { error: noop, warn: noop, log: noop, - addPrefix: noop + setLoggerId: () => NOOP_LOGGER }; + +const LOGGERS: Record = { + noop: NOOP_LOGGER, + json: new JSONLogger(), + plain: new ConsoleLogger(), + error: new AlwaysStdErrLogger() +} as const; + +export function getLogger(format: LoggerFormat): Logger { + return LOGGERS[format]; +} diff --git a/src/common/models/app-settings/app-settings.ts b/src/common/models/app-settings/app-settings.ts index 870cffc25..42b3f7641 100644 --- a/src/common/models/app-settings/app-settings.ts +++ b/src/common/models/app-settings/app-settings.ts @@ -16,6 +16,7 @@ */ import { Executor } from "plywood"; +import { Logger } from "../../logger/logger"; import { ClientCustomization, Customization, @@ -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 { @@ -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 { diff --git a/src/common/models/cluster/cluster.fixtures.ts b/src/common/models/cluster/cluster.fixtures.ts index abc669838..4c49e83c1 100644 --- a/src/common/models/cluster/cluster.fixtures.ts +++ b/src/common/models/cluster/cluster.fixtures.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { NOOP_LOGGER } from "../../logger/logger"; import { Cluster, ClusterJS, fromConfig } from "./cluster"; export class ClusterFixtures { @@ -33,7 +34,7 @@ export class ClusterFixtures { } static druidWikiCluster(): Cluster { - return fromConfig(ClusterFixtures.druidWikiClusterJS()); + return fromConfig(ClusterFixtures.druidWikiClusterJS(), NOOP_LOGGER); } static druidTwitterClusterJS(): ClusterJS { @@ -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 { @@ -68,6 +69,6 @@ export class ClusterFixtures { guardDataCubes, introspectionStrategy: "segment-metadata-fallback" - }); + }, NOOP_LOGGER); } } diff --git a/src/common/models/cluster/cluster.mocha.ts b/src/common/models/cluster/cluster.mocha.ts index 576638121..2809a47b9 100644 --- a/src/common/models/cluster/cluster.mocha.ts +++ b/src/common/models/cluster/cluster.mocha.ts @@ -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"; @@ -31,7 +32,7 @@ function buildCluster(options: Partial = {}): Cluster { return fromConfig({ ...baseConfig, ...options - }); + }, NOOP_LOGGER); } use(equivalent); @@ -39,7 +40,7 @@ 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", @@ -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", () => { @@ -119,7 +120,7 @@ describe("Cluster", () => { }); it("should read request decorator", () => { - const cluster = fromConfig({ + const cluster = buildCluster({ name: "foobar", url: "http://foobar", requestDecorator: { @@ -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); diff --git a/src/common/models/cluster/cluster.ts b/src/common/models/cluster/cluster.ts index afd7f9cf5..bf3b2f310 100644 --- a/src/common/models/cluster/cluster.ts +++ b/src/common/models/cluster/cluster.ts @@ -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"; @@ -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); @@ -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, @@ -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); diff --git a/src/common/models/customization/customization.mocha.ts b/src/common/models/customization/customization.mocha.ts index e0754d9bc..7acdfc5c9 100644 --- a/src/common/models/customization/customization.mocha.ts +++ b/src/common/models/customization/customization.mocha.ts @@ -18,41 +18,51 @@ import { expect } from "chai"; import { Timezone } from "chronoshift"; import * as sinon from "sinon"; +import { NOOP_LOGGER } from "../../logger/logger"; import { DEFAULT_COLORS, DEFAULT_MAIN_COLOR, DEFAULT_SERIES_COLORS } from "../colors/colors"; import * as localeModule from "../locale/locale"; import * as urlShortenerModule from "../url-shortener/url-shortener"; -import { Customization, DEFAULT_TIMEZONES, DEFAULT_TITLE, fromConfig, serialize } from "./customization"; +import { + Customization, + CustomizationJS, + DEFAULT_TIMEZONES, + DEFAULT_TITLE, + fromConfig, + serialize +} from "./customization"; import { customization, customizationJS } from "./customization.fixtures"; +const build = (customization: CustomizationJS) => fromConfig(customization, NOOP_LOGGER); + describe("Customization", () => { describe("fromConfig", () => { it("should pass headerBackground", () => { - const customization = fromConfig({ ...customizationJS, headerBackground: "foobar" }); + const customization = build({ ...customizationJS, headerBackground: "foobar" }); expect(customization).to.contain({ headerBackground: "foobar" }); }); it("should pass customLogoSvg", () => { - const customization = fromConfig({ ...customizationJS, customLogoSvg: "foobar" }); + const customization = build({ ...customizationJS, customLogoSvg: "foobar" }); expect(customization).to.contain({ customLogoSvg: "foobar" }); }); it("should pass sentryDSN", () => { - const customization = fromConfig({ ...customizationJS, sentryDSN: "foobar" }); + const customization = build({ ...customizationJS, sentryDSN: "foobar" }); expect(customization).to.contain({ sentryDSN: "foobar" }); }); describe("title", () => { it("should pass title", () => { - const customization = fromConfig({ ...customizationJS, title: "foobar" }); + const customization = build({ ...customizationJS, title: "foobar" }); expect(customization).to.contain({ title: "foobar" }); }); it("should use default title", () => { - const customization = fromConfig({ ...customizationJS, title: undefined }); + const customization = build({ ...customizationJS, title: undefined }); expect(customization).to.contain({ title: DEFAULT_TITLE }); }); @@ -60,7 +70,7 @@ describe("Customization", () => { describe("timezones", () => { it("should create timezone objects from strings", () => { - const customization = fromConfig({ + const customization = build({ ...customizationJS, timezones: ["Europe/Warsaw", "Asia/Manila"] }); @@ -71,7 +81,7 @@ describe("Customization", () => { }); it("should use default timezones if empty", () => { - const customization = fromConfig({ ...customizationJS, timezones: undefined }); + const customization = build({ ...customizationJS, timezones: undefined }); expect(customization.timezones).to.deep.equal(DEFAULT_TIMEZONES); }); @@ -95,13 +105,13 @@ describe("Customization", () => { }); it("should call urlShortener fromConfig", () => { - fromConfig({ urlShortener: { input: 42 } } as any); + build({ urlShortener: { input: 42 } } as any); expect(urlShortenerFromConfig.calledWith({ input: 42 })).to.be.true; }); it("should use result of urlShortener fromConfig", () => { - const settings = fromConfig({ urlShortener: { input: 42 } } as any); + const settings = build({ urlShortener: { input: 42 } } as any); expect(settings).to.deep.contain({ urlShortener: "foobar" @@ -123,13 +133,13 @@ describe("Customization", () => { }); it("should call locale fromConfig", () => { - fromConfig({ locale: { input: 42 } } as any); + build({ locale: { input: 42 } } as any); expect(localeFromConfig.calledWith({ input: 42 })).to.be.true; }); it("should use result of locale fromConfig", () => { - const settings = fromConfig({ locale: { input: 42 } } as any); + const settings = build({ locale: { input: 42 } } as any); expect(settings).to.deep.contain({ locale: "foobar" @@ -139,13 +149,13 @@ describe("Customization", () => { describe("cssVariables", () => { it("should create empty object as default", () => { - const customization = fromConfig({ ...customizationJS, cssVariables: undefined }); + const customization = build({ ...customizationJS, cssVariables: undefined }); expect(customization).to.deep.contain({ cssVariables: {} }); }); it("should pass only valid variables", () => { - const customization = fromConfig({ + const customization = build({ ...customizationJS, cssVariables: { "brand": "foobar", @@ -165,13 +175,13 @@ describe("Customization", () => { describe("visualizationColors", () => { it("should set default colors if no colors are defined", () => { - const customization = fromConfig({ ...customizationJS, visualizationColors: undefined }); + const customization = build({ ...customizationJS, visualizationColors: undefined }); expect(customization).to.deep.contain({ visualizationColors: DEFAULT_COLORS }); }); it("should override default main color when property is defined", () => { - const customization = fromConfig({ + const customization = build({ ...customizationJS, visualizationColors: { main: "foobar-color" } @@ -186,7 +196,7 @@ describe("Customization", () => { }); it("should override default series colors when property is defined", () => { - const customization = fromConfig({ + const customization = build({ ...customizationJS, visualizationColors: { series: ["one fish", "two fish", "red fish", "blue fish"] } diff --git a/src/common/models/customization/customization.ts b/src/common/models/customization/customization.ts index 22b743058..242363361 100644 --- a/src/common/models/customization/customization.ts +++ b/src/common/models/customization/customization.ts @@ -16,10 +16,10 @@ */ import { Timezone } from "chronoshift"; -import { LOGGER } from "../../logger/logger"; +import { Logger } from "../../logger/logger"; import { assoc } from "../../utils/functional/functional"; import { isNil, isTruthy } from "../../utils/general/general"; -import { DEFAULT_COLORS, DEFAULT_MAIN_COLOR, DEFAULT_SERIES_COLORS, VisualizationColors } from "../colors/colors"; +import { DEFAULT_COLORS, VisualizationColors } from "../colors/colors"; import { ExternalView, ExternalViewValue } from "../external-view/external-view"; import { fromConfig as localeFromConfig, Locale, LocaleJS, serialize as serializeLocale } from "../locale/locale"; import { fromConfig as urlShortenerFromConfig, UrlShortener, UrlShortenerDef } from "../url-shortener/url-shortener"; @@ -160,12 +160,12 @@ export interface ClientCustomization { visualizationColors: VisualizationColors; } -function verifyCssVariables(cssVariables: Record): CssVariables { +function verifyCssVariables(cssVariables: Record, logger: Logger): CssVariables { return Object.keys(cssVariables) .filter(variableName => { const valid = availableCssVariables.indexOf(variableName) > -1; if (!valid) { - LOGGER.warn(`Unsupported css variables "${variableName}" found.`); + logger.warn(`Unsupported css variables "${variableName}" found.`); } return valid; }) @@ -180,7 +180,7 @@ function readVisualizationColors(config: CustomizationJS): VisualizationColors { return { ...DEFAULT_COLORS, ...config.visualizationColors }; } -export function fromConfig(config: CustomizationJS = {}): Customization { +export function fromConfig(config: CustomizationJS = {}, logger: Logger): Customization { const { title = DEFAULT_TITLE, headerBackground, @@ -209,10 +209,10 @@ export function fromConfig(config: CustomizationJS = {}): Customization { headerBackground, customLogoSvg, sentryDSN, - cssVariables: verifyCssVariables(cssVariables), + cssVariables: verifyCssVariables(cssVariables, logger), urlShortener: urlShortenerFromConfig(urlShortener), timezones, - locale: localeFromConfig(locale), + locale: localeFromConfig(locale, logger), messages, externalViews, visualizationColors diff --git a/src/common/models/data-cube/data-cube.mocha.ts b/src/common/models/data-cube/data-cube.mocha.ts index 11601a3cb..3d4d5b2e7 100644 --- a/src/common/models/data-cube/data-cube.mocha.ts +++ b/src/common/models/data-cube/data-cube.mocha.ts @@ -19,25 +19,30 @@ import { expect, use } from "chai"; import { $, AttributeInfo } from "plywood"; import { SinonSpy, spy } from "sinon"; import equivalent from "../../../client/utils/test-utils/equivalent"; +import { NOOP_LOGGER } from "../../logger/logger"; import { deduceAttributes } from "../../utils/external/datacube-to-external"; -import { fromConfig as clusterFromConfig } from "../cluster/cluster"; +import { Cluster, fromConfig as clusterFromConfig } from "../cluster/cluster"; import { createDimension, DimensionJS, timeDimension } from "../dimension/dimension"; import { allDimensions, fromConfig as dimensionsFromConfig } from "../dimension/dimensions"; import { DataCube, DataCubeJS, fromConfig } from "./data-cube"; import { addAttributes } from "./queryable-data-cube"; +const logger = NOOP_LOGGER; + +const build = (config: DataCubeJS, cluster: Cluster = undefined) => fromConfig(config, cluster, logger); + use(equivalent); describe("DataCube", () => { const druidCluster = clusterFromConfig({ name: "druid", url: "http://driud" - }); + }, logger); describe("validates", () => { it("throws an error if bad name is used", () => { expect(() => { - fromConfig({ + build({ name: "wiki hello", clusterName: "druid", source: "wiki", @@ -64,7 +69,7 @@ describe("DataCube", () => { it("throws an error if the defaultSortMeasure can not be found", () => { expect(() => { - fromConfig({ + build({ name: "wiki", clusterName: "druid", source: "wiki", @@ -92,7 +97,7 @@ describe("DataCube", () => { it("throws an error if duplicate name is used across measures and dimensions", () => { expect(() => { - fromConfig({ + build({ name: "wiki", clusterName: "druid", source: "wiki", @@ -119,7 +124,7 @@ describe("DataCube", () => { it("throws an error if duplicate name is used in measures", () => { expect(() => { - fromConfig({ + build({ name: "wiki", clusterName: "druid", source: "wiki", @@ -150,7 +155,7 @@ describe("DataCube", () => { it("throws an error if duplicate name is used in dimensions", () => { expect(() => { - fromConfig({ + build({ name: "wiki", clusterName: "druid", source: "wiki", @@ -220,7 +225,7 @@ describe("DataCube", () => { } }; - const dataCube = fromConfig(legacyDataCubeJS, druidCluster); + const dataCube = build(legacyDataCubeJS, druidCluster); expect(dataCube).to.deep.equal({ attributeOverrides: [ @@ -283,7 +288,7 @@ describe("DataCube", () => { describe("#deduceAttributes", () => { it("works in a generic case", () => { - const dataCube = fromConfig({ + const dataCube = build({ name: "wiki", clusterName: "druid", source: "wiki", @@ -353,7 +358,7 @@ describe("DataCube", () => { }); it("omits unsupported expressions", () => { - const dataCube = fromConfig({ + const dataCube = build({ name: "wiki", clusterName: "druid", source: "wiki", @@ -400,7 +405,7 @@ describe("DataCube", () => { }); describe("#addAttributes", () => { - const dataCubeStub = fromConfig({ + const dataCubeStub = build({ name: "wiki", title: "Wiki", clusterName: "druid", @@ -640,7 +645,7 @@ describe("DataCube", () => { { name: "deleted", type: "NUMBER" } ]); - const dataCubeWithDim = fromConfig({ + const dataCubeWithDim = build({ name: "wiki", title: "Wiki", clusterName: "druid", @@ -670,7 +675,7 @@ describe("DataCube", () => { }); describe("#addAttributes (new dim)", () => { - const dataCube = fromConfig({ + const dataCube = build({ name: "wiki", title: "Wiki", clusterName: "druid", @@ -726,41 +731,41 @@ describe("DataCube", () => { }; describe("timeAttribute property warnings", () => { - let consoleWarnSpy: SinonSpy; + let loggerWarnSpy: SinonSpy; beforeEach(() => { - consoleWarnSpy = spy(console, "warn"); + loggerWarnSpy = spy(logger, "warn"); }); afterEach(() => { - consoleWarnSpy.restore(); + loggerWarnSpy.restore(); }); it("should warn if timeAttribute is missing", () => { - fromConfig({ ...baseCube }, druidCluster); - expect(consoleWarnSpy.args[0][0]).to.be.equal("DataCube \"wiki\" should have property timeAttribute. Setting timeAttribute to default value \"__time\""); + build({ ...baseCube }, druidCluster); + expect(loggerWarnSpy.args[0][0]).to.be.equal("DataCube \"wiki\" should have property timeAttribute. Setting timeAttribute to default value \"__time\""); }); it("should warn if timeAttribute has different value than \"__time\"", () => { - fromConfig({ ...baseCube, timeAttribute: "foobar" }, druidCluster); - expect(consoleWarnSpy.args[0][0]).to.be.equal('timeAttribute in DataCube "wiki" should have value "__time" because it is required by Druid. Overriding timeAttribute to "__time"'); + build({ ...baseCube, timeAttribute: "foobar" }, druidCluster); + expect(loggerWarnSpy.args[0][0]).to.be.equal('timeAttribute in DataCube "wiki" should have value "__time" because it is required by Druid. Overriding timeAttribute to "__time"'); }); }); it("should add timeAttribute", () => { - const cube = fromConfig({ ...baseCube, dimensions: [timeDimensionJS] }, druidCluster); + const cube = build({ ...baseCube, dimensions: [timeDimensionJS] }, druidCluster); expect(cube.timeAttribute).to.be.equivalent($("__time")); }); it("should prepend time dimension if not defined", () => { - const cube = fromConfig({ ...baseCube, dimensions: [] }, druidCluster); + const cube = build({ ...baseCube, dimensions: [] }, druidCluster); const timeAttribute = $("__time"); expect(cube.dimensions.byName.time).to.be.deep.equal(timeDimension(timeAttribute)); expect(cube.dimensions.tree).to.be.deep.equal(["time"]); }); it("should override invalid time Attribute", () => { - const cube = fromConfig({ ...baseCube, timeAttribute: "foobar" }, druidCluster); + const cube = build({ ...baseCube, timeAttribute: "foobar" }, druidCluster); const timeAttribute = $("__time"); expect(cube.timeAttribute).to.be.equivalent(timeAttribute); }); @@ -780,11 +785,11 @@ describe("DataCube", () => { }; it("should throw without timeAttribute property", () => { - expect(() => fromConfig({ ...baseCube })).to.throw("DataCube \"medals\" must have defined timeAttribute property"); + expect(() => build({ ...baseCube })).to.throw("DataCube \"medals\" must have defined timeAttribute property"); }); it("should pass well defined dimensions and timeAttribute", () => { - const cube = fromConfig({ ...baseCube, timeAttribute: "time_column", dimensions: [timeDimensionJS] }); + const cube = build({ ...baseCube, timeAttribute: "time_column", dimensions: [timeDimensionJS] }); const timeAttribute = $("time_column"); expect(cube.timeAttribute).to.be.equivalent(timeAttribute); expect(cube.dimensions).to.be.deep.equal(dimensionsFromConfig([timeDimensionJS])); diff --git a/src/common/models/data-cube/data-cube.ts b/src/common/models/data-cube/data-cube.ts index 8a46c6b4d..ce71c858a 100644 --- a/src/common/models/data-cube/data-cube.ts +++ b/src/common/models/data-cube/data-cube.ts @@ -30,6 +30,7 @@ import { External, RefExpression } from "plywood"; +import { Logger } from "../../logger/logger"; import { isTruthy, quoteNames, verifyUrlSafeName } from "../../utils/general/general"; import { Cluster } from "../cluster/cluster"; import { Dimension, DimensionKind, timeDimension as createTimeDimension } from "../dimension/dimension"; @@ -279,14 +280,14 @@ function readAttributes(config: DataCubeJS): Pick; } -export function fromClusterAndExternal(name: string, cluster: Cluster, external: External): QueryableDataCube { +export function fromClusterAndExternal(name: string, cluster: Cluster, external: External, logger: Logger): QueryableDataCube { const dataCube = fromConfig({ name, clusterName: cluster.name, source: String(external.source), refreshRule: RefreshRule.query().toJS() - }, cluster); + }, cluster, logger); return attachExternalExecutor(dataCube, external); } diff --git a/src/common/models/external-view/external-view.ts b/src/common/models/external-view/external-view.ts index d314ccf08..11a4c5609 100644 --- a/src/common/models/external-view/external-view.ts +++ b/src/common/models/external-view/external-view.ts @@ -72,8 +72,7 @@ export class ExternalView implements Instance fromConfig(locale, NOOP_LOGGER); const en_us = LOCALES["en-US"]; describe("locale", () => { describe("fromConfig", () => { it("should return default locale if passed undefined", () => { - const locale = fromConfig(); + const locale = build(undefined); expect(locale).to.deep.equal(en_us); }); it("should use base locale", () => { - const locale = fromConfig({ base: "en-US", overrides: {} }); + const locale = build({ base: "en-US", overrides: {} }); expect(locale).to.deep.equal(en_us); }); it("should return default locale if passed unrecognized base identifier", () => { - const locale = fromConfig({ base: "foobar" } as any); + const locale = build({ base: "foobar" } as any); expect(locale).to.deep.equal(en_us); }); it("should use base locale and override desired fields", () => { - const locale = fromConfig({ base: "en-US", overrides: { weekStart: 42 } }); + const locale = build({ base: "en-US", overrides: { weekStart: 42 } }); expect(locale).to.deep.equal({ ...en_us, diff --git a/src/common/models/locale/locale.ts b/src/common/models/locale/locale.ts index 0ffe34b56..980c5072c 100644 --- a/src/common/models/locale/locale.ts +++ b/src/common/models/locale/locale.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { LOGGER } from "../../logger/logger"; +import { Logger } from "../../logger/logger"; import { isObject, isTruthy } from "../../utils/general/general"; const enUS: Locale = { @@ -46,11 +46,11 @@ export interface Locale { exportEncoding: string; } -export function fromConfig(locale?: LocaleJS): Locale { +export function fromConfig(locale: LocaleJS, logger: Logger): Locale { if (!isObject(locale)) return DEFAULT_LOCALE; const { base, overrides } = locale; if (!isTruthy(LOCALES[base])) { - LOGGER.warn(`Unsupported locale identifier: ${base}. Fallback to en-US.`); + logger.warn(`Unsupported locale identifier: ${base}. Fallback to en-US.`); return DEFAULT_LOCALE; } return { diff --git a/src/common/models/sources/sources.ts b/src/common/models/sources/sources.ts index ec4120b54..cf46006b8 100644 --- a/src/common/models/sources/sources.ts +++ b/src/common/models/sources/sources.ts @@ -15,6 +15,7 @@ */ import { NamedArray } from "immutable-class"; +import { Logger } from "../../logger/logger"; import { isTruthy } from "../../utils/general/general"; import { ClientCluster, @@ -61,13 +62,13 @@ interface ClustersConfig { brokerHost?: string; } -function readClusters({ clusters, druidHost, brokerHost }: ClustersConfig): Cluster[] { - if (Array.isArray(clusters)) return clusters.map(clusterFromConfig); +function readClusters({ clusters, druidHost, brokerHost }: ClustersConfig, logger: Logger): Cluster[] { + if (Array.isArray(clusters)) return clusters.map(cluster => clusterFromConfig(cluster, logger)); if (isTruthy(druidHost) || isTruthy(brokerHost)) { return [clusterFromConfig({ name: "druid", url: druidHost || brokerHost - })]; + }, logger)]; } return []; } @@ -77,17 +78,17 @@ interface DataCubesConfig { dataSources?: DataCubeJS[]; } -function readDataCubes({ dataCubes, dataSources }: DataCubesConfig, clusters: Cluster[]): DataCube[] { +function readDataCubes({ dataCubes, dataSources }: DataCubesConfig, clusters: Cluster[], logger: Logger): DataCube[] { const cubes = dataCubes || dataSources || []; return cubes.map(cube => { const cluster = findCluster(cube, clusters); - return dataCubeFromConfig(cube, cluster); + return dataCubeFromConfig(cube, cluster, logger); }); } -export function fromConfig(config: SourcesJS): Sources { - const clusters = readClusters(config); - const dataCubes = readDataCubes(config, clusters); +export function fromConfig(config: SourcesJS, logger: Logger): Sources { + const clusters = readClusters(config, logger); + const dataCubes = readDataCubes(config, clusters, logger); return { clusters, diff --git a/src/common/utils/time-monitor/time-monitor.ts b/src/common/utils/time-monitor/time-monitor.ts index 689dd89e3..0ccadd947 100644 --- a/src/common/utils/time-monitor/time-monitor.ts +++ b/src/common/utils/time-monitor/time-monitor.ts @@ -30,7 +30,7 @@ export class TimeMonitor { private doingChecks = false; constructor(logger: Logger) { - this.logger = logger.addPrefix("TimeMonitor"); + this.logger = logger.setLoggerId("TimeMonitor"); this.checks = new Map(); this.timekeeper = Timekeeper.EMPTY; setInterval(this.doChecks, 1000); @@ -58,7 +58,7 @@ export class TimeMonitor { logger.log(`Got the latest time for '${name}' (${updatedTime.toISOString()})`); this.timekeeper = this.timekeeper.updateTime(name, updatedTime); }).catch(e => { - logger.error(`Failed getting time for '${name}', using previous time.`, `Error: ${e.message}`); + logger.error(`Failed getting time for '${name}', using previous time. Error: ${e.message}`); this.timekeeper = this.timekeeper.updateTime(name, previousTime); } ); diff --git a/src/common/utils/time/time.ts b/src/common/utils/time/time.ts index f7c4f6de6..5321f77bb 100644 --- a/src/common/utils/time/time.ts +++ b/src/common/utils/time/time.ts @@ -206,3 +206,7 @@ export function validateISOTime(time: string): boolean { export function combineDateAndTimeIntoMoment(date: string, time: string, timezone: Timezone): Moment { return tz(`${date}T${time}`, timezone.toString()); } + +export function isoNow(): string { + return (new Date()).toISOString(); +} diff --git a/src/common/utils/yaml-helper/yaml-helper.ts b/src/common/utils/yaml-helper/yaml-helper.ts index 921a68d26..d66bdb0db 100644 --- a/src/common/utils/yaml-helper/yaml-helper.ts +++ b/src/common/utils/yaml-helper/yaml-helper.ts @@ -16,6 +16,7 @@ */ import { AttributeInfo } from "plywood"; +import { Logger } from "../../logger/logger"; import { AppSettings } from "../../models/app-settings/app-settings"; import { Cluster, DEFAULT_INTROSPECTION_STRATEGY, @@ -92,13 +93,13 @@ function yamlPropAdder(lines: string[], withComments: boolean, options: PropAdde } } -function getYamlPropAdder(object: any, labels: any, lines: string[], withComments = false) { +function getYamlPropAdder(object: any, labels: any, lines: string[], withComments: boolean, logger: Logger) { const adder = (propName: string, additionalOptions?: { defaultValue?: any }) => { const propVerbiage = labels[propName]; let comment: string; if (!propVerbiage) { - console.warn(`No labels for ${propName}, please fix this in 'common/models/labels.ts'`); + logger.warn(`No labels for ${propName}, please fix this in 'common/models/labels.ts'`); comment = ""; } else { comment = propVerbiage.description; @@ -116,11 +117,11 @@ function getYamlPropAdder(object: any, labels: any, lines: string[], withComment return { add: adder }; } -function customizationToYAML(customization: Customization, withComments: boolean): string[] { +function customizationToYAML(customization: Customization, withComments: boolean, logger: Logger): string[] { const { timezones, externalViews, cssVariables } = customization; const lines: string[] = []; - getYamlPropAdder(customization, CUSTOMIZATION, lines, withComments) + getYamlPropAdder(customization, CUSTOMIZATION, lines, withComments, logger) .add("customLogoSvg") .add("headerBackground") .add("sentryDSN") @@ -153,12 +154,12 @@ function customizationToYAML(customization: Customization, withComments: boolean return lines.map(line => `${pad}${line}`); } -function clusterToYAML(cluster: Cluster, withComments: boolean): string[] { +function clusterToYAML(cluster: Cluster, withComments: boolean, logger: Logger): string[] { const lines: string[] = [ `name: ${cluster.name}` ]; - const props = getYamlPropAdder(cluster, CLUSTER, lines, withComments); + const props = getYamlPropAdder(cluster, CLUSTER, lines, withComments, logger); props .add("url") @@ -256,7 +257,7 @@ function sourceToYAML(source: Source): string { return yamlArray(source); } -function dataCubeToYAML(dataCube: DataCube, withComments: boolean): string[] { +function dataCubeToYAML(dataCube: DataCube, withComments: boolean, logger: Logger): string[] { let lines: string[] = [ `name: ${dataCube.name}`, `title: ${dataCube.title}`, @@ -281,7 +282,7 @@ function dataCubeToYAML(dataCube: DataCube, withComments: boolean): string[] { } lines.push(""); - const addProps = getYamlPropAdder(dataCube, DATA_CUBE, lines, withComments); + const addProps = getYamlPropAdder(dataCube, DATA_CUBE, lines, withComments, logger); addProps .add("defaultTimezone", { defaultValue: DEFAULT_DEFAULT_TIMEZONE }) @@ -469,7 +470,7 @@ export function printExtra(extra: Extra, withComments: boolean): string { return lines.join("\n"); } -export function sourcesToYaml(sources: Sources, withComments: boolean): string { +export function sourcesToYaml(sources: Sources, withComments: boolean, logger: Logger): string { const { dataCubes, clusters } = sources; if (!dataCubes.length) throw new Error("Could not find any data cubes, please verify network connectivity"); @@ -478,16 +479,16 @@ export function sourcesToYaml(sources: Sources, withComments: boolean): string { if (clusters.length) { lines.push("clusters:"); - lines = lines.concat.apply(lines, clusters.map(c => clusterToYAML(c, withComments))); + lines = lines.concat.apply(lines, clusters.map(c => clusterToYAML(c, withComments, logger))); } lines.push("dataCubes:"); - lines = lines.concat.apply(lines, dataCubes.map(d => dataCubeToYAML(d, withComments))); + lines = lines.concat.apply(lines, dataCubes.map(d => dataCubeToYAML(d, withComments, logger))); return lines.join("\n"); } -export function appSettingsToYaml(settings: AppSettings, withComments: boolean): string { +export function appSettingsToYaml(settings: AppSettings, withComments: boolean, logger: Logger): string { // TODO: shouldn't we print oauth and clientTimeout? const { customization, oauth, clientTimeout } = settings; @@ -495,7 +496,7 @@ export function appSettingsToYaml(settings: AppSettings, withComments: boolean): if (customization) { lines.push("customization:"); - lines.push(...customizationToYAML(customization, withComments)); + lines.push(...customizationToYAML(customization, withComments, logger)); } return lines.join("\n"); diff --git a/src/server/app.ts b/src/server/app.ts index 6a42ecfd4..d5f679210 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -107,7 +107,7 @@ export default function createApp(serverSettings: ServerSettings, settingsManage serverSettings, settingsManager.appSettings, settingsManager.sourcesGetter, - settingsManager.logger.addPrefix(name)); + settingsManager.logger.setLoggerId(name)); } catch (e) { settingsManager.logger.warn(`Plugin ${name} threw an error: ${e.message}`); } @@ -141,17 +141,17 @@ export default function createApp(serverSettings: ServerSettings, settingsManage attachRouter("/", express.static(join(__dirname, "../../build/public"))); attachRouter("/", express.static(join(__dirname, "../../assets"))); - attachRouter(serverSettings.readinessEndpoint, readinessRouter(settingsManager.sourcesGetter)); + attachRouter(serverSettings.readinessEndpoint, readinessRouter(settingsManager)); attachRouter(serverSettings.livenessEndpoint, livenessRouter); // Data routes - attachRouter("/sources", sourcesRouter(settingsManager.sourcesGetter)); + attachRouter("/sources", sourcesRouter(settingsManager)); attachRouter("/plywood", plywoodRouter(settingsManager)); - attachRouter("/plyql", plyqlRouter(settingsManager.sourcesGetter)); - attachRouter("/mkurl", mkurlRouter(settingsManager.appSettings, settingsManager.sourcesGetter)); - attachRouter("/shorten", shortenRouter(settingsManager.appSettings, isTrustedProxy)); + attachRouter("/plyql", plyqlRouter(settingsManager)); + attachRouter("/mkurl", mkurlRouter(settingsManager)); + attachRouter("/shorten", shortenRouter(settingsManager, isTrustedProxy)); - attachRouter("/", turniloRouter(settingsManager.appSettings, () => settingsManager.getTimekeeper(), version)); + attachRouter("/", turniloRouter(settingsManager, version)); // Catch 404 and redirect to / app.use((req: Request, res: Response) => { diff --git a/src/server/cli.ts b/src/server/cli.ts index 9b8b27fc2..0f60671e3 100644 --- a/src/server/cli.ts +++ b/src/server/cli.ts @@ -20,6 +20,7 @@ import buildSettings, { settingsForDatasetFile, settingsForDruidConnection } fro import printIntrospectedSettings from "./cli/introspect-cluster"; import { loadConfigFile } from "./cli/load-config-file"; import { + loggerOption, passwordOption, portOption, serverHostOption, @@ -48,16 +49,18 @@ program .description("Runs Turnilo using config file") .argument("", "Path to config file") .addOption(portOption) + .addOption(loggerOption) .addOption(serverRootOption) .addOption(serverHostOption) .addOption(usernameOption) .addOption(passwordOption) .addOption(verboseOption) - .action((configPath, { username, password, serverRoot, serverHost, port, verbose }) => { + .action((configPath, { username, password, loggerFormat, serverRoot, serverHost, port, verbose }) => { const anchorPath = path.dirname(configPath); const auth = parseCredentials(username, password, "http-basic"); const config = loadConfigFile(configPath, program); const options = { + loggerFormat, serverRoot, serverHost, verbose, @@ -77,14 +80,15 @@ program .command("run-examples") .description("Runs Turnilo with example datasets") .addOption(portOption) + .addOption(loggerOption) .addOption(serverRootOption) .addOption(serverHostOption) .addOption(verboseOption) - .action(({ port, verbose, serverRoot, serverHost }) => { + .action(({ port, verbose, loggerFormat, serverRoot, serverHost }) => { const configPath = path.join(__dirname, "../../config-examples.yaml"); const anchorPath = path.dirname(configPath); const config = loadConfigFile(configPath, program); - const options = { port, verbose, serverHost, serverRoot }; + const options = { port, verbose, serverHost, serverRoot, loggerFormat }; runTurnilo( buildSettings(config, options), @@ -100,14 +104,15 @@ program .description("Runs turnilo that connects to Druid cluster and introspects it for datasets") .argument("", "Url of Druid cluster") .addOption(portOption) + .addOption(loggerOption) .addOption(serverRootOption) .addOption(serverHostOption) .addOption(verboseOption) .addOption(usernameOption) .addOption(passwordOption) - .action((url, { port, verbose, username, password, serverRoot, serverHost }) => { + .action((url, { port, verbose, username, password, serverRoot, serverHost, loggerFormat }) => { const auth = parseCredentials(username, password, "http-basic"); - const options = { port, verbose, serverHost, serverRoot }; + const options = { port, verbose, serverHost, serverRoot, loggerFormat }; runTurnilo( settingsForDruidConnection(url, options, auth), process.cwd(), @@ -123,11 +128,13 @@ program .argument("", "Path to json file with data") .requiredOption("-t, --time-attribute ", "JSON field name with time column") .addOption(portOption) + .addOption(loggerOption) .addOption(serverRootOption) .addOption(serverHostOption) .addOption(verboseOption) - .action((file, { timeAttribute, port, verbose, serverHost, serverRoot }) => { + .action((file, { timeAttribute, port, verbose, serverHost, serverRoot, loggerFormat }) => { const options = { + loggerFormat, serverRoot, serverHost, verbose, diff --git a/src/server/cli/build-settings.mocha.ts b/src/server/cli/build-settings.mocha.ts index d5a1ee0bd..29e723aa3 100644 --- a/src/server/cli/build-settings.mocha.ts +++ b/src/server/cli/build-settings.mocha.ts @@ -18,8 +18,9 @@ import { expect, use } from "chai"; import { RefExpression } from "plywood"; import sinon from "sinon"; import equivalent from "../../client/utils/test-utils/equivalent"; +import { NOOP_LOGGER } from "../../common/logger/logger"; import * as SourcesModule from "../../common/models/app-settings/app-settings"; -import { EMPTY_APP_SETTINGS } from "../../common/models/app-settings/app-settings"; +import { emptySettings } from "../../common/models/app-settings/app-settings"; import { fromConfig } from "../../common/models/cluster/cluster"; import * as AppSettingsModule from "../../common/models/sources/sources"; import { ServerSettings } from "../models/server-settings/server-settings"; @@ -37,7 +38,7 @@ describe("Build Settings", () => { }); it("should create empty app settings", () => { - expect(settings.appSettings).to.be.deep.equal(EMPTY_APP_SETTINGS); + expect(settings.appSettings).to.be.deep.equal(emptySettings(NOOP_LOGGER)); }); it("should create empty cluster array", () => { @@ -89,7 +90,7 @@ describe("Build Settings", () => { }); it("should create empty app settings", () => { - expect(settings.appSettings).to.be.deep.equal(EMPTY_APP_SETTINGS); + expect(settings.appSettings).to.be.deep.equal(emptySettings(NOOP_LOGGER)); }); it("should create empty data cubes array", () => { @@ -149,7 +150,7 @@ describe("Build Settings", () => { }); it("should pass auth object to all clusters", () => { - const makeCluster = (name: string) => fromConfig({ name, url: `https://${name}.com` }); + const makeCluster = (name: string) => fromConfig({ name, url: `https://${name}.com` }, NOOP_LOGGER); const settings = buildSettings({ clusters: [makeCluster("foobar-1"), makeCluster("foobar-2")] diff --git a/src/server/cli/build-settings.ts b/src/server/cli/build-settings.ts index a104a46fe..d385cbe9f 100644 --- a/src/server/cli/build-settings.ts +++ b/src/server/cli/build-settings.ts @@ -15,13 +15,14 @@ */ import path from "path"; -import { EMPTY_APP_SETTINGS, fromConfig as appSettingsFromConfig } from "../../common/models/app-settings/app-settings"; +import { getLogger } from "../../common/logger/logger"; +import { emptySettings, fromConfig as appSettingsFromConfig } from "../../common/models/app-settings/app-settings"; import { ClusterAuthJS } from "../../common/models/cluster-auth/cluster-auth"; import { fromConfig as clusterFromConfig } from "../../common/models/cluster/cluster"; import { fromConfig as dataCubeFromConfig } from "../../common/models/data-cube/data-cube"; import { fromConfig as sourcesFromConfig, Sources, SourcesJS } from "../../common/models/sources/sources"; import { isNil } from "../../common/utils/general/general"; -import { ServerSettings, ServerSettingsJS } from "../models/server-settings/server-settings"; +import { LoggerFormat, ServerSettings, ServerSettingsJS } from "../models/server-settings/server-settings"; import { TurniloSettings } from "./run-turnilo"; export interface ServerOptions { @@ -29,6 +30,7 @@ export interface ServerOptions { verbose?: boolean; serverHost?: string; serverRoot?: string; + loggerFormat?: LoggerFormat; } function overrideClustersAuth(config: SourcesJS, auth: ClusterAuthJS): SourcesJS { @@ -48,9 +50,10 @@ export default function buildSettings(config: object, options: ServerOptions, au ...options }; const serverSettings = ServerSettings.fromJS(serverSettingsJS); - const appSettings = appSettingsFromConfig(config); + const logger = getLogger(serverSettings.loggerFormat); + const appSettings = appSettingsFromConfig(config, logger); const sourcesJS = isNil(auth) ? config : overrideClustersAuth(config, auth); - const sources = sourcesFromConfig(sourcesJS); + const sources = sourcesFromConfig(sourcesJS, logger); return { serverSettings, @@ -60,16 +63,17 @@ export default function buildSettings(config: object, options: ServerOptions, au } export function settingsForDruidConnection(url: string, options: ServerOptions, auth?: ClusterAuthJS): TurniloSettings { + const serverSettings = ServerSettings.fromJS(options); + const logger = getLogger(serverSettings.loggerFormat); const sources: Sources = { dataCubes: [], clusters: [clusterFromConfig({ name: "druid", url, auth - })] + }, logger)] }; - const appSettings = EMPTY_APP_SETTINGS; - const serverSettings = ServerSettings.fromJS(options); + const appSettings = emptySettings(getLogger(serverSettings.loggerFormat)); return { sources, @@ -79,17 +83,18 @@ export function settingsForDruidConnection(url: string, options: ServerOptions, } export function settingsForDatasetFile(datasetPath: string, timeAttribute: string, options: ServerOptions): TurniloSettings { + const serverSettings = ServerSettings.fromJS(options); + const logger = getLogger(serverSettings.loggerFormat); const sources: Sources = { dataCubes: [dataCubeFromConfig({ name: path.basename(datasetPath, path.extname(datasetPath)), clusterName: "native", source: datasetPath, timeAttribute - }, undefined)], + }, undefined, logger)], clusters: [] }; - const appSettings = EMPTY_APP_SETTINGS; - const serverSettings = ServerSettings.fromJS(options); + const appSettings = emptySettings(getLogger(serverSettings.loggerFormat)); return { sources, diff --git a/src/server/cli/create-server.ts b/src/server/cli/create-server.ts index 3bfa8752d..efc394855 100644 --- a/src/server/cli/create-server.ts +++ b/src/server/cli/create-server.ts @@ -17,9 +17,10 @@ import { Command } from "commander"; import { Express } from "express"; import http from "http"; import { AddressInfo } from "net"; +import { Logger } from "../../common/logger/logger"; import { ServerSettings } from "../models/server-settings/server-settings"; -export default function createServer(serverSettings: ServerSettings, app: Express, program: Command) { +export default function createServer(serverSettings: ServerSettings, app: Express, logger: Logger, program: Command) { const server = http.createServer(app); @@ -45,7 +46,7 @@ export default function createServer(serverSettings: ServerSettings, app: Expres server.on("listening", () => { const address = server.address() as AddressInfo; - console.log(`Turnilo is listening on address ${address.address} port ${address.port}`); + logger.log(`Turnilo is listening on address ${address.address} port ${address.port}`); }); app.set("port", serverSettings.port); diff --git a/src/server/cli/introspect-cluster.ts b/src/server/cli/introspect-cluster.ts index 83e160ea6..34752ebef 100644 --- a/src/server/cli/introspect-cluster.ts +++ b/src/server/cli/introspect-cluster.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { NULL_LOGGER } from "../../common/logger/logger"; import { appSettingsToYaml, printExtra, sourcesToYaml } from "../../common/utils/yaml-helper/yaml-helper"; import { SettingsManager } from "../utils/settings-manager/settings-manager"; import { TurniloSettings } from "./run-turnilo"; @@ -27,7 +26,7 @@ export default function printIntrospectedSettings( const settingsManager = new SettingsManager(appSettings, sources, { anchorPath: process.cwd(), initialLoadTimeout: serverSettings.pageMustLoadTimeout, - logger: NULL_LOGGER + logger: "error" }); return settingsManager.getFreshSources({ @@ -38,8 +37,8 @@ export default function printIntrospectedSettings( header: true, version }, withComments), - appSettingsToYaml(appSettings, withComments), - sourcesToYaml(sources, withComments) + appSettingsToYaml(appSettings, withComments, settingsManager.logger), + sourcesToYaml(sources, withComments, settingsManager.logger) ]; process.stdout.write(config.join("\n")); diff --git a/src/server/cli/options.ts b/src/server/cli/options.ts index 8f1cb0a78..a4f2c2c1c 100644 --- a/src/server/cli/options.ts +++ b/src/server/cli/options.ts @@ -15,7 +15,7 @@ */ import { Option } from "commander"; -import { DEFAULT_PORT } from "../models/server-settings/server-settings"; +import { DEFAULT_LOGGER_FORMAT, DEFAULT_PORT, LOGGER_FORMAT_VALUES } from "../models/server-settings/server-settings"; import { parseInteger } from "./utils"; export const portOption = new Option( @@ -23,14 +23,21 @@ export const portOption = new Option( `Port number to start server on. Default: ${DEFAULT_PORT}` ).argParser(parseInteger); +export const loggerOption = new Option( + "--logger-format ", + `Format for logger. Default: ${DEFAULT_LOGGER_FORMAT}` +).choices(LOGGER_FORMAT_VALUES); + export const serverRootOption = new Option( "--server-root ", "Custom path to act as turnilo root" ); + export const serverHostOption = new Option( "--server-host ", "Host that server will bind to" ); + export const verboseOption = new Option( "--verbose", "Verbose mode" @@ -40,6 +47,7 @@ export const usernameOption = new Option( "--username ", "Username that will be used in HTTP Basic authentication to Druid cluster" ); + export const passwordOption = new Option( "--password ", "Password that will be used in HTTP Basic authentication to Druid cluster" diff --git a/src/server/cli/run-turnilo.ts b/src/server/cli/run-turnilo.ts index 9beca95e5..24af3dd27 100644 --- a/src/server/cli/run-turnilo.ts +++ b/src/server/cli/run-turnilo.ts @@ -15,7 +15,6 @@ */ import { Command } from "commander"; -import { LOGGER } from "../../common/logger/logger"; import { AppSettings } from "../../common/models/app-settings/app-settings"; import { Sources } from "../../common/models/sources/sources"; import createApp from "../app"; @@ -41,7 +40,7 @@ export default function runTurnilo( anchorPath, initialLoadTimeout: serverSettings.pageMustLoadTimeout, verbose, - logger: LOGGER + logger: serverSettings.loggerFormat }); - createServer(serverSettings, createApp(serverSettings, settingsManager, version), program); + createServer(serverSettings, createApp(serverSettings, settingsManager, version), settingsManager.logger, program); } diff --git a/src/server/models/server-settings/server-settings.ts b/src/server/models/server-settings/server-settings.ts index 944c77aeb..d7f44f01f 100644 --- a/src/server/models/server-settings/server-settings.ts +++ b/src/server/models/server-settings/server-settings.ts @@ -22,6 +22,7 @@ import { PluginSettings } from "../plugin-settings/plugin-settings"; export type Iframe = "allow" | "deny"; export type TrustProxy = "none" | "always"; export type StrictTransportSecurity = "none" | "always"; +export type LoggerFormat = "plain" | "json" | "noop" | "error"; export interface ServerSettingsValue { port?: number; @@ -37,6 +38,7 @@ export interface ServerSettingsValue { trustProxy?: TrustProxy; strictTransportSecurity?: StrictTransportSecurity; plugins?: PluginSettings[]; + loggerFormat?: LoggerFormat; } export type ServerSettingsJS = ServerSettingsValue & { @@ -58,6 +60,8 @@ const TRUST_PROXY_VALUES: TrustProxy[] = ["none", "always"]; const DEFAULT_TRUST_PROXY: TrustProxy = "none"; const STRICT_TRANSPORT_SECURITY_VALUES: StrictTransportSecurity[] = ["none", "always"]; const DEFAULT_STRICT_TRANSPORT_SECURITY: StrictTransportSecurity = "none"; +export const DEFAULT_LOGGER_FORMAT: LoggerFormat = "plain"; +export const LOGGER_FORMAT_VALUES: LoggerFormat[] = ["plain", "json"]; const defaultServerSettings: ServerSettingsValue = { iframe: DEFAULT_IFRAME, @@ -72,7 +76,8 @@ const defaultServerSettings: ServerSettingsValue = { serverTimeout: DEFAULT_SERVER_TIMEOUT, strictTransportSecurity: DEFAULT_STRICT_TRANSPORT_SECURITY, trustProxy: DEFAULT_TRUST_PROXY, - verbose: false + verbose: false, + loggerFormat: DEFAULT_LOGGER_FORMAT }; export class ServerSettings extends Record(defaultServerSettings) { @@ -87,11 +92,13 @@ export class ServerSettings extends Record(defaultServerSet requestLogFormat, serverRoot, serverTimeout, - serverHost + serverHost, + loggerFormat } = parameters; optionalEnsureOneOf(iframe, IFRAME_VALUES, "ServerSettings: iframe"); optionalEnsureOneOf(trustProxy, TRUST_PROXY_VALUES, "ServerSettings: trustProxy"); optionalEnsureOneOf(strictTransportSecurity, STRICT_TRANSPORT_SECURITY_VALUES, "ServerSettings: strictTransportSecurity"); + optionalEnsureOneOf(loggerFormat, LOGGER_FORMAT_VALUES, "ServerSettings: loggerFormat"); const readinessEndpoint = !parameters.readinessEndpoint && !!parameters.healthEndpoint ? parameters.healthEndpoint : parameters.readinessEndpoint; const verbose = Boolean(parameters.verbose); @@ -99,6 +106,7 @@ export class ServerSettings extends Record(defaultServerSet return new ServerSettings({ port: typeof parameters.port === "string" ? parseInt(parameters.port, 10) : parameters.port, + loggerFormat, plugins, readinessEndpoint, livenessEndpoint, diff --git a/src/server/routes/mkurl/mkurl.mocha.ts b/src/server/routes/mkurl/mkurl.mocha.ts index 7e02950b3..61c7bdbb0 100644 --- a/src/server/routes/mkurl/mkurl.mocha.ts +++ b/src/server/routes/mkurl/mkurl.mocha.ts @@ -30,7 +30,10 @@ const app = express(); app.use(bodyParser.json()); -app.use(mkurlPath, mkurlRouter(appSettings, () => Promise.resolve(wikiSourcesWithExecutor))); +app.use(mkurlPath, mkurlRouter({ + appSettings, + getSources: () => Promise.resolve(wikiSourcesWithExecutor) +})); describe("mkurl router", () => { it("gets a simple url back", (testComplete: any) => { diff --git a/src/server/routes/mkurl/mkurl.ts b/src/server/routes/mkurl/mkurl.ts index 076d0f6a2..36d424e98 100644 --- a/src/server/routes/mkurl/mkurl.ts +++ b/src/server/routes/mkurl/mkurl.ts @@ -24,7 +24,7 @@ import { Essence } from "../../../common/models/essence/essence"; import { getDataCube, Sources } from "../../../common/models/sources/sources"; import { urlHashConverter } from "../../../common/utils/url-hash-converter/url-hash-converter"; import { definitionConverters, ViewDefinitionVersion } from "../../../common/view-definitions"; -import { SourcesGetter } from "../../utils/settings-manager/settings-manager"; +import { SettingsManager } from "../../utils/settings-manager/settings-manager"; function convertToClientAppSettings(appSettings: AppSettings): ClientAppSettings { return deserialize(serialize(appSettings)); @@ -37,7 +37,7 @@ function convertToClientDataCube(cube: QueryableDataCube): ClientDataCube { }; } -export function mkurlRouter(appSettings: AppSettings, sourcesGetter: SourcesGetter) { +export function mkurlRouter(settings: Pick) { const router = Router(); @@ -69,7 +69,7 @@ export function mkurlRouter(appSettings: AppSettings, sourcesGetter: SourcesGett let sources: Sources; try { - sources = await sourcesGetter(); + sources = await settings.getSources(); } catch (e) { res.status(400).send({ error: "Couldn't load settings" }); return; @@ -86,7 +86,7 @@ export function mkurlRouter(appSettings: AppSettings, sourcesGetter: SourcesGett } const clientDataCube = convertToClientDataCube(myDataCube); - const clientAppSettings = convertToClientAppSettings(appSettings); + const clientAppSettings = convertToClientAppSettings(settings.appSettings); let essence: Essence; try { diff --git a/src/server/routes/plyql/plyql.mocha.ts b/src/server/routes/plyql/plyql.mocha.ts index 3da154c1d..324fbc90b 100644 --- a/src/server/routes/plyql/plyql.mocha.ts +++ b/src/server/routes/plyql/plyql.mocha.ts @@ -26,7 +26,7 @@ const app = express(); app.use(bodyParser.json()); -app.use("/", plyqlRouter(() => Promise.resolve(wikiSourcesWithExecutor))); +app.use("/", plyqlRouter({ getSources: () => Promise.resolve(wikiSourcesWithExecutor) })); const pageQuery = "SELECT SUM(added) as Added FROM `wiki` GROUP BY page ORDER BY Added DESC LIMIT 10;"; const timeQuery = "SELECT TIME_BUCKET(time, 'PT1H', 'Etc/UTC') as TimeByHour, SUM(added) as Added FROM `wiki` GROUP BY 1 ORDER BY TimeByHour ASC"; diff --git a/src/server/routes/plyql/plyql.ts b/src/server/routes/plyql/plyql.ts index b925ec07c..625704750 100644 --- a/src/server/routes/plyql/plyql.ts +++ b/src/server/routes/plyql/plyql.ts @@ -19,7 +19,7 @@ import { Request, Response, Router } from "express"; import { $, Dataset, Expression, RefExpression } from "plywood"; import { isQueryable } from "../../../common/models/data-cube/queryable-data-cube"; import { getDataCube } from "../../../common/models/sources/sources"; -import { SourcesGetter } from "../../utils/settings-manager/settings-manager"; +import { SettingsManager } from "../../utils/settings-manager/settings-manager"; interface PlyqlOutputFunctions { [key: string]: (data: Dataset) => string; @@ -35,7 +35,7 @@ const outputFunctions: PlyqlOutputFunctions = { tsv: (data: Dataset): string => data.toTSV() }; -export function plyqlRouter(sourcesGetter: SourcesGetter) { +export function plyqlRouter(settings: Pick) { const router = Router(); @@ -81,7 +81,7 @@ export function plyqlRouter(sourcesGetter: SourcesGetter) { }); try { - const sources = await sourcesGetter(); + const sources = await settings.getSources(); const myDataCube = getDataCube(sources, dataCube); if (!myDataCube) { diff --git a/src/server/routes/plywood/plywood.mocha.ts b/src/server/routes/plywood/plywood.mocha.ts index 0d5487687..8bf10014f 100644 --- a/src/server/routes/plywood/plywood.mocha.ts +++ b/src/server/routes/plywood/plywood.mocha.ts @@ -19,12 +19,14 @@ import * as bodyParser from "body-parser"; import express from "express"; import { $ } from "plywood"; import supertest from "supertest"; +import { NOOP_LOGGER } from "../../../common/logger/logger"; import { wikiSourcesWithExecutor } from "../../../common/models/sources/sources.fixtures"; import { plywoodRouter } from "./plywood"; const settingsManagerFixture = { getSources: () => Promise.resolve(wikiSourcesWithExecutor), - anchorPath: "." + anchorPath: ".", + logger: NOOP_LOGGER }; const app = express(); diff --git a/src/server/routes/plywood/plywood.ts b/src/server/routes/plywood/plywood.ts index c985b4efa..599a2f6ef 100644 --- a/src/server/routes/plywood/plywood.ts +++ b/src/server/routes/plywood/plywood.ts @@ -18,15 +18,15 @@ import { Timezone } from "chronoshift"; import { Request, Response, Router } from "express"; import { Dataset, Expression } from "plywood"; -import { LOGGER } from "../../../common/logger/logger"; +import { errorToMessage } from "../../../common/logger/logger"; import { isQueryable } from "../../../common/models/data-cube/queryable-data-cube"; import { getDataCube } from "../../../common/models/sources/sources"; import { checkAccess } from "../../utils/datacube-guard/datacube-guard"; import { loadQueryDecorator } from "../../utils/query-decorator-loader/load-query-decorator"; import { SettingsManager } from "../../utils/settings-manager/settings-manager"; -export function plywoodRouter(settingsManager: Pick) { - +export function plywoodRouter(settingsManager: Pick) { + const logger = settingsManager.logger; const router = Router(); router.post("/", async (req: Request, res: Response) => { @@ -89,7 +89,7 @@ export function plywoodRouter(settingsManager: Pick { describe("single druid cluster", () => { before(done => { app = express(); - app.use("/", readinessRouter(() => Promise.resolve(wikiSources))); + app.use("/", readinessRouter({ getSources: () => Promise.resolve(wikiSources), logger: NOOP_LOGGER })); server = app.listen(0, done); }); @@ -71,7 +72,7 @@ describe("readiness router", () => { describe("multiple druid clusters", () => { before(done => { app = express(); - app.use("/", readinessRouter(() => Promise.resolve(wikiTwitterSources))); + app.use("/", readinessRouter({ getSources: () => Promise.resolve(wikiTwitterSources), logger: NOOP_LOGGER })); server = app.listen(0, done); }); @@ -80,7 +81,7 @@ describe("readiness router", () => { }); const multipleClustersTests: any[] = [ - { + { scenario: "all healthy brokers", wikiBroker: { status: 200, initialized: true, delay: 0 }, twitterBroker: { status: 200, initialized: true, delay: 0 }, diff --git a/src/server/routes/readiness/readiness.ts b/src/server/routes/readiness/readiness.ts index 6f743b31d..36f58371d 100644 --- a/src/server/routes/readiness/readiness.ts +++ b/src/server/routes/readiness/readiness.ts @@ -17,9 +17,9 @@ import { Request, Response, Router } from "express"; import * as request from "request-promise-native"; -import { LOGGER } from "../../../common/logger/logger"; +import { Logger } from "../../../common/logger/logger"; import { Cluster } from "../../../common/models/cluster/cluster"; -import { SourcesGetter } from "../../utils/settings-manager/settings-manager"; +import { SettingsManager } from "../../utils/settings-manager/settings-manager"; const unhealthyHttpStatus = 503; const healthyHttpStatus = 200; @@ -75,27 +75,28 @@ function aggregateHealthStatus(clusterHealths: ClusterHealth[]): ClusterHealthSt return isSomeUnhealthy ? ClusterHealthStatus.unhealthy : ClusterHealthStatus.healthy; } -function logUnhealthy(clusterHealths: ClusterHealth[]): void { +function logUnhealthy(logger: Logger, clusterHealths: ClusterHealth[]): void { const unhealthyClusters = clusterHealths.filter(({ status }) => status === ClusterHealthStatus.unhealthy); unhealthyClusters.forEach(({ message, url }: ClusterHealth) => { - LOGGER.log(`Unhealthy cluster url: ${url}. Message: ${message}`); + logger.log(`Unhealthy cluster url: ${url}. Message: ${message}`); }); } -export function readinessRouter(getSources: SourcesGetter) { +export function readinessRouter(settings: Pick) { + const logger = settings.logger; const router = Router(); router.get("/", async (req: Request, res: Response) => { try { - const sources = await getSources(); + const sources = await settings.getSources(); const clusterHealths = await checkClusters(sources.clusters); - logUnhealthy(clusterHealths); + logUnhealthy(logger, clusterHealths); const overallHealthStatus = aggregateHealthStatus(clusterHealths); const httpState = statusToHttpStatus(overallHealthStatus); res.status(httpState).send({ status: overallHealthStatus, clusters: clusterHealths }); } catch (reason) { - LOGGER.log(`Readiness check error: ${reason.message}`); + logger.warn(`Readiness check error: ${reason.message}`); res.status(unhealthyHttpStatus).send({ status: ClusterHealthStatus.unhealthy, message: reason.message }); } }); diff --git a/src/server/routes/shorten/shorten.mocha.ts b/src/server/routes/shorten/shorten.mocha.ts index acf864d21..bebc8f721 100644 --- a/src/server/routes/shorten/shorten.mocha.ts +++ b/src/server/routes/shorten/shorten.mocha.ts @@ -19,6 +19,8 @@ import express from "express"; import { Express } from "express"; import * as http from "http"; import supertest from "supertest"; +import { NOOP_LOGGER } from "../../../common/logger/logger"; +import { appSettings } from "../../../common/models/app-settings/app-settings.fixtures"; import { fromConfig } from "../../../common/models/customization/customization"; import { UrlShortenerDef } from "../../../common/models/url-shortener/url-shortener"; import { FailUrlShortenerJS, SuccessUrlShortenerJS } from "../../../common/models/url-shortener/url-shortener.fixtures"; @@ -27,9 +29,13 @@ import { shortenRouter } from "./shorten"; const shortenPath = "/shorten"; const settingsFactory = (urlShortener: UrlShortenerDef) => ({ - customization: fromConfig({ - urlShortener - }) + logger: NOOP_LOGGER, + appSettings: { + ...appSettings, + customization: fromConfig({ + urlShortener + }, NOOP_LOGGER) + } }); const callShortener = (app: Express) => supertest(app) diff --git a/src/server/routes/shorten/shorten.ts b/src/server/routes/shorten/shorten.ts index 3d229c2e1..922738d63 100644 --- a/src/server/routes/shorten/shorten.ts +++ b/src/server/routes/shorten/shorten.ts @@ -16,17 +16,19 @@ import { Request, Response, Router } from "express"; import * as request from "request-promise-native"; -import { AppSettings } from "../../../common/models/app-settings/app-settings"; +import { errorToMessage } from "../../../common/logger/logger"; import { UrlShortenerContext } from "../../../common/models/url-shortener/url-shortener"; +import { SettingsManager } from "../../utils/settings-manager/settings-manager"; -export function shortenRouter(settings: Pick, isTrustedProxy: boolean) { +export function shortenRouter(settings: Pick, isTrustedProxy: boolean) { + const logger = settings.logger; const router = Router(); router.get("/", async (req: Request, res: Response) => { const { url } = req.query; try { - const shortener = settings.customization.urlShortener; + const shortener = settings.appSettings.customization.urlShortener; const context: UrlShortenerContext = { // If trust proxy is not enabled, app is understood as directly facing the internet clientIp: isTrustedProxy ? req.ip : req.connection.remoteAddress @@ -34,10 +36,8 @@ export function shortenRouter(settings: Pick, isTr const shortUrl = await shortener(request, url as string, context); res.json({ shortUrl }); } catch (error) { - console.log("error:", error.message); - if (error.hasOwnProperty("stack")) { - console.log((error as any).stack); - } + logger.error(errorToMessage(error)); + res.status(500).send({ error: "could not shorten url", message: error.message diff --git a/src/server/routes/sources/sources.ts b/src/server/routes/sources/sources.ts index 83f36283b..fd62ef964 100644 --- a/src/server/routes/sources/sources.ts +++ b/src/server/routes/sources/sources.ts @@ -15,31 +15,29 @@ */ import { Request, Response, Router } from "express"; -import { LOGGER } from "../../../common/logger/logger"; +import { errorToMessage } from "../../../common/logger/logger"; import { serialize } from "../../../common/models/sources/sources"; import { checkAccess } from "../../utils/datacube-guard/datacube-guard"; -import { SourcesGetter } from "../../utils/settings-manager/settings-manager"; +import { SettingsManager } from "../../utils/settings-manager/settings-manager"; -export function sourcesRouter(sourcesGetter: SourcesGetter) { +export function sourcesRouter(settings: Pick) { - const logger = LOGGER.addPrefix("Settings Endpoint: "); + const logger = settings.logger.setLoggerId("Sources"); const router = Router(); router.get("/", async (req: Request, res: Response) => { try { - const { clusters, dataCubes } = await sourcesGetter(); + const { clusters, dataCubes } = await settings.getSources(); res.json(serialize({ clusters, dataCubes: dataCubes.filter( dataCube => checkAccess(dataCube, req.headers) ) })); } catch (error) { - logger.error(error.message); - if (error.hasOwnProperty("stack")) { - logger.error(error.stack); - } - res.status(500).send({ + logger.error(errorToMessage(error)); + + res.status(500).send({ error: "Can't fetch settings", message: error.message }); diff --git a/src/server/routes/turnilo/turnilo.ts b/src/server/routes/turnilo/turnilo.ts index e6b02d811..bd13c0545 100644 --- a/src/server/routes/turnilo/turnilo.ts +++ b/src/server/routes/turnilo/turnilo.ts @@ -16,13 +16,12 @@ */ import { Request, Response, Router } from "express"; -import { AppSettings } from "../../../common/models/app-settings/app-settings"; import { getTitle } from "../../../common/models/customization/customization"; -import { Timekeeper } from "../../../common/models/timekeeper/timekeeper"; -import { Nullary } from "../../../common/utils/functional/functional"; +import { SettingsManager } from "../../utils/settings-manager/settings-manager"; import { mainLayout } from "../../views"; -export function turniloRouter(appSettings: AppSettings, getTimekeeper: Nullary, version: string) { +export function turniloRouter(settings: Pick, version: string) { + const appSettings = settings.appSettings; const router = Router(); @@ -32,7 +31,7 @@ export function turniloRouter(appSettings: AppSettings, getTimekeeper: Nullary