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

Bug 1712285 - Tag all log messages #411

Merged
merged 4 commits into from
Jun 10, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

[Full changelog](https://github.com/mozilla/glean.js/compare/v0.15.0...main)

* [#411](https://github.com/mozilla/glean.js/pull/411): Tag all messages logged by Glean with the component they are coming from.
* [#399](https://github.com/mozilla/glean.js/pull/399): Check if there are ping data before attempting to delete it.
* This change lowers the amount of log messages related to attempting to delete inexistent data.

Expand Down
24 changes: 14 additions & 10 deletions glean/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import * as path from "path";
import { argv, platform } from "process";
import { promisify } from "util";

import log, { LoggingLevel } from "./core/log.js";

const LOG_TAG = "CLI";

// The name of the directory which will contain the Python virtual environment
// used to run the glean-parser.
const VIRTUAL_ENVIRONMENT_DIR = ".venv";
Expand Down Expand Up @@ -100,7 +104,7 @@ function getPythonVenvBinariesPath(venvRoot: string): string {
* is accessible, `false` otherwise.
*/
async function checkPythonVenvExists(venvPath: string): Promise<boolean> {
console.log(`Checking for a Glean virtual environment at ${venvPath}`);
log(LOG_TAG, `Checking for a Glean virtual environment at ${venvPath}`);

const venvPython =
path.join(getPythonVenvBinariesPath(venvPath), getSystemPythonBinName());
Expand All @@ -124,7 +128,7 @@ async function checkPythonVenvExists(venvPath: string): Promise<boolean> {
* @returns `true` if the environment was correctly created, `false` otherwise.
*/
async function createPythonVenv(venvPath: string): Promise<boolean> {
console.log(`Creating a Glean virtual environment at ${venvPath}`);
log(LOG_TAG, `Creating a Glean virtual environment at ${venvPath}`);

const pipFilename = (platform === "win32") ? "pip3.exe" : "pip3";
const venvPip =
Expand All @@ -140,10 +144,10 @@ async function createPythonVenv(venvPath: string): Promise<boolean> {
});

stopSpinner(spinner);
console.log(`${stdout}`);
log(LOG_TAG, `${stdout}`);

if (err) {
console.error(`${stderr}`);
log(LOG_TAG, `${stderr}`);
return false;
}
}
Expand All @@ -162,9 +166,9 @@ async function setup(projectRoot: string) {

const venvExists = await checkPythonVenvExists(venvRoot);
if (venvExists) {
console.log(`Using Glean virtual environment at ${venvRoot}`);
log(LOG_TAG, `Using Glean virtual environment at ${venvRoot}`);
} else if (!await createPythonVenv(venvRoot)){
console.error(`Failed to create a Glean virtual environment at ${venvRoot}`);
log(LOG_TAG, `Failed to create a Glean virtual environment at ${venvRoot}`);
process.exit(1);
}
}
Expand All @@ -186,10 +190,10 @@ async function runGlean(projectRoot: string, parserArgs: string[]) {
});

stopSpinner(spinner);
console.log(`${stdout}`);
log(LOG_TAG, `${stdout}`);

if (err) {
console.error(`${stderr}`);
log(LOG_TAG, `${stderr}`);
process.exit(1);
}
}
Expand Down Expand Up @@ -232,7 +236,7 @@ async function run(args: string[]) {
try {
await setup(projectRoot);
} catch (err) {
console.error("Failed to setup the Glean build environment", err);
log(LOG_TAG, ["Failed to setup the Glean build environment.\n", err], LoggingLevel.Error);
process.exit(1);
}

Expand All @@ -241,6 +245,6 @@ async function run(args: string[]) {

// For discoverability, try to leave this function as the last one on this file.
run(argv).catch(e => {
console.error("There was an error running Glean", e);
log(LOG_TAG, ["There was an error running Glean.\n", e], LoggingLevel.Error);
process.exit(1);
});
25 changes: 20 additions & 5 deletions glean/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import type Plugin from "../plugins/index.js";
import { validateHeader, validateURL } from "./utils.js";
import type Uploader from "./upload/uploader.js";
import type { DebugOptions } from "./debug_options.js";
import log, { LoggingLevel } from "./log.js";

const LOG_TAG = "core.Config";

/**
* Describes how to configure Glean.
Expand Down Expand Up @@ -69,23 +72,35 @@ export class Configuration implements ConfigurationInterface {
static validateDebugViewTag(tag: string): boolean {
const validation = validateHeader(tag);
if (!validation) {
console.error(
`"${tag}" is not a valid \`debugViewTag\` value.`,
"Please make sure the value passed satisfies the regex `^[a-zA-Z0-9-]{1,20}$`."
log(
LOG_TAG,
[
`"${tag}" is not a valid \`debugViewTag\` value.`,
"Please make sure the value passed satisfies the regex `^[a-zA-Z0-9-]{1,20}$`."
],
LoggingLevel.Error
);
}
return validation;
}

static validateSourceTags(tags: string[]): boolean {
if (tags.length < 1 || tags.length > GLEAN_MAX_SOURCE_TAGS) {
console.error(`A list of tags cannot contain more than ${GLEAN_MAX_SOURCE_TAGS} elements.`);
log(
LOG_TAG,
`A list of tags cannot contain more than ${GLEAN_MAX_SOURCE_TAGS} elements.`,
LoggingLevel.Error
);
return false;
}

for (const tag of tags) {
if (tag.startsWith("glean")) {
console.error("Tags starting with `glean` are reserved and must not be used.");
log(
LOG_TAG,
"Tags starting with `glean` are reserved and must not be used.",
LoggingLevel.Error
);
return false;
}

Expand Down
28 changes: 22 additions & 6 deletions glean/src/core/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import log, { LoggingLevel } from "./log.js";

const LOG_TAG = "core.Dispatcher";

// The possible states a dispatcher instance can be in.
export const enum DispatcherState {
// The dispatcher has not been initialized yet.
Expand Down Expand Up @@ -84,7 +88,7 @@ class Dispatcher {
try {
await task();
} catch(e) {
console.error("Error executing task:", e);
log(LOG_TAG, ["Error executing task:", e], LoggingLevel.Error);
}
}

Expand Down Expand Up @@ -144,9 +148,13 @@ class Dispatcher {
}
})
.catch(error => {
console.error(
"IMPOSSIBLE: Something went wrong while the dispatcher was executing the tasks queue.",
error
log(
LOG_TAG,
[
"IMPOSSIBLE: Something went wrong while the dispatcher was executing the tasks queue.",
error
],
LoggingLevel.Error
);
});
}
Expand All @@ -169,7 +177,11 @@ class Dispatcher {
private launchInternal(command: Command, priorityTask = false): boolean {
if (!priorityTask && this.state === DispatcherState.Uninitialized) {
if (this.queue.length >= this.maxPreInitQueueSize) {
console.warn("Unable to enqueue task, pre init queue is full.");
log(
LOG_TAG,
"Unable to enqueue task, pre init queue is full.",
LoggingLevel.Warn
);
return false;
}
}
Expand Down Expand Up @@ -213,7 +225,11 @@ class Dispatcher {
*/
flushInit(task?: Task): void {
if (this.state !== DispatcherState.Uninitialized) {
console.warn("Attempted to initialize the Dispatcher, but it is already initialized. Ignoring.");
log(
LOG_TAG,
"Attempted to initialize the Dispatcher, but it is already initialized. Ignoring.",
LoggingLevel.Warn
);
return;
}

Expand Down
14 changes: 13 additions & 1 deletion glean/src/core/error/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ import type { MetricType } from "../metrics/index.js";
import type { ErrorType } from "./error_type.js";
import CounterMetricType from "../metrics/types/counter.js";
import { combineIdentifierAndLabel, stripLabel } from "../metrics/types/labeled.js";
import log from "../log.js";

/**
* Create a log tag for a specific metric type.
*
* @param metric The metric type to create a tag for.
* @returns The tag.
*/
function createLogTag(metric: MetricType): string {
const capitalizedType = metric.type.charAt(0).toUpperCase() + metric.type.slice(1);
return `core.Metrics.${capitalizedType}`;
}

/**
* For a given metric, get the metric in which to record errors.
Expand Down Expand Up @@ -54,7 +66,7 @@ export default class ErrorManager {
numErrors = 1
): Promise<void> {
const errorMetric = getErrorMetricForMetric(metric, error);
console.warn(`${metric.baseIdentifier()}: ${message}`);
log(createLogTag(metric), `${metric.baseIdentifier()}: ${message}`);
if (numErrors > 0) {
await CounterMetricType._private_addUndispatched(errorMetric, numErrors);
} else {
Expand Down
15 changes: 11 additions & 4 deletions glean/src/core/events/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import type Plugin from "../../plugins/index.js";
import log, { LoggingLevel } from "../log.js";

import type { PingPayload } from "../pings/ping_payload.js";
import type { JSONObject } from "../utils.js";

const LOG_TAG = "core.Events";

export class CoreEvent<
// An array of arguments that the event will provide as context to the plugin action.
Context extends unknown[] = unknown[],
Expand All @@ -29,10 +32,14 @@ export class CoreEvent<
*/
registerPlugin(plugin: Plugin<CoreEvent<Context, Result>>): void {
if (this.plugin) {
console.error(
`Attempted to register plugin '${plugin.name}', which listens to the event '${plugin.event}'.`,
`That event is already watched by plugin '${this.plugin.name}'`,
`Plugin '${plugin.name}' will be ignored.`
log(
LOG_TAG,
[
`Attempted to register plugin '${plugin.name}', which listens to the event '${plugin.event}'.`,
`That event is already watched by plugin '${this.plugin.name}'`,
`Plugin '${plugin.name}' will be ignored.`
],
LoggingLevel.Error
);
return;
}
Expand Down
13 changes: 10 additions & 3 deletions glean/src/core/events/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
import type { CoreEvent } from "./index.js";
import CoreEvents from "./index.js";
import type Plugin from "../../plugins/index.js";
import log, { LoggingLevel } from "../log.js";

const LOG_TAG = "core.Events.Utils";

/**
* Registers a plugin to the desired Glean event.
Expand All @@ -21,9 +24,13 @@ export function registerPluginToEvent<E extends CoreEvent>(plugin: Plugin<E>): v
return;
}

console.error(
`Attempted to register plugin '${plugin.name}', which listens to the event '${plugin.event}'.`,
"That is not a valid Glean event. Ignoring"
log(
LOG_TAG,
[
`Attempted to register plugin '${plugin.name}', which listens to the event '${plugin.event}'.`,
"That is not a valid Glean event. Ignoring"
],
LoggingLevel.Error
);
}

Expand Down
40 changes: 30 additions & 10 deletions glean/src/core/glean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import TestPlatform from "../platform/test/index.js";
import { Lifetime } from "./metrics/lifetime.js";
import { Context } from "./context.js";
import PingType from "./pings/ping_type.js";
import log, { LoggingLevel } from "./log.js";

const LOG_TAG = "core.Glean";

class Glean {
// The Glean singleton.
Expand Down Expand Up @@ -179,17 +182,29 @@ class Glean {
config?: ConfigurationInterface
): void {
if (Context.initialized) {
console.warn("Attempted to initialize Glean, but it has already been initialized. Ignoring.");
log(
LOG_TAG,
"Attempted to initialize Glean, but it has already been initialized. Ignoring.",
LoggingLevel.Warn
);
return;
}

if (applicationId.length === 0) {
console.error("Unable to initialize Glean, applicationId cannot be an empty string.");
log(
LOG_TAG,
"Unable to initialize Glean, applicationId cannot be an empty string.",
LoggingLevel.Error
);
return;
}

if (!Glean.instance._platform) {
console.error("Unable to initialize Glean, environment has not been set.");
log(
LOG_TAG,
"Unable to initialize Glean, environment has not been set.",
LoggingLevel.Error
);
return;
}

Expand Down Expand Up @@ -316,10 +331,14 @@ class Glean {
static setUploadEnabled(flag: boolean): void {
Context.dispatcher.launch(async () => {
if (!Context.initialized) {
console.error(
"Changing upload enabled before Glean is initialized is not supported.\n",
"Pass the correct state into `Glean.initialize\n`.",
"See documentation at https://mozilla.github.io/glean/book/user/general-api.html#initializing-the-glean-sdk`"
log(
LOG_TAG,
[
"Changing upload enabled before Glean is initialized is not supported.\n",
"Pass the correct state into `Glean.initialize\n`.",
"See documentation at https://mozilla.github.io/glean/book/user/general-api.html#initializing-the-glean-sdk`"
],
LoggingLevel.Error
);
return;
}
Expand Down Expand Up @@ -362,7 +381,7 @@ class Glean {
*/
static setDebugViewTag(value: string): void {
if (!Configuration.validateDebugViewTag(value)) {
console.error(`Invalid \`debugViewTag\` ${value}. Ignoring.`);
log(LOG_TAG, `Invalid \`debugViewTag\` ${value}. Ignoring.`, LoggingLevel.Error);
return;
}

Expand All @@ -388,7 +407,7 @@ class Glean {
*/
static setSourceTags(value: string[]): void {
if (!Configuration.validateSourceTags(value)) {
console.error(`Invalid \`sourceTags\` ${value.toString()}. Ignoring.`);
log(LOG_TAG, `Invalid \`sourceTags\` ${value.toString()}. Ignoring.`, LoggingLevel.Error);
return;
}

Expand Down Expand Up @@ -426,7 +445,8 @@ class Glean {
// we log a debug message about the change.
if (Glean.instance._platform && Glean.instance._platform.name !== platform.name) {
// TODO: Only show this message outside of test mode, and rephrase it as an error (depends on Bug 1682771).
console.debug(
log(
LOG_TAG,
`Changing Glean platform from "${Glean.platform.name}" to "${platform.name}".`,
);
}
Expand Down
Loading