maraisr / diary Goto Github PK

I have found myself wanting to consume logs in a server runtime as json. This helps me consume/query/filter logs in a far easier manner, than parsing the messages.

To do this, and w/o forcing this on users I will expose a new entrypoint diary/middleware .

The idea being, that here we'll the suite of hooks?? (might not) as well as a collection of "baked-in" plugins, like json and maybe request logger. etc.

Maybe api:

import { json } from 'diary/middleware';
import { diary } from 'diary';

const logger = diary('');

json(); // will assume global
json(logger); // will just log as json for `logger`

Describe the bug

After 78334b3 the types in https://github.com/maraisr/diary/blob/a723e0b203321faa6a95195c9991c2cf74c67427/src/index.d.ts are broken. Specifically, a type definition for LogFnAsError is now missing.

This means that any use of customLogger.error(...) and customLogger.fatal(...) in a typescript project doesn't work as expected.

To reproduce

import { diary } from 'diary';

const customLogger = diary('');

customLogger.error(new Error('test'));

Typescript can't see any type definition for LogFnAsError and treats it as any. When using @typescript-eslint/eslint-plugin in a project, a lint error for the rule @typescript-eslint/no-unsafe-call is emitted for customLogger.error.

Expected behavior

customLogger.error(...) and customLogger.fatal(...) are typed correctly.

In v1.0.2, the setLevel api was removed. In the release notes it is mentioned that this can be replaced by using the new onEmit api. However, as far as I can tell, this cannot express the common requirement of "print all messages from all loggers above a certain loglevel". (If there is a way to do this easily, I'd love to hear it)

Conceptually, I see 3 ways in which this could work.

• reintroduce the "setLevel" api
• when creating a diary, also create :error, :warning etc variants, and have the logging methods use those, so you can express
  "*:warning,*:error" in the filter string
• allow inheritance of diaries, something along the lines of this:

const customEmitter = (logevent: LogEvent) => { /* logic here */ };
const baseDiary = diary("root", customEmitter);
const myDiary = baseDiary.diary("myDiary"); // its name would then be "root:myDiary"
// myDiary would then automatically have the same emitter logic

I'm speaking out of line here, but to me the last option feels like the most natural fit with the current api

Turns out localStorage is not always accessible in the browser. In my particular usecase, my app was used in an iframe, and chrome with restrictive 3rd party cookie settings throws an error when trying to access localstorage. This would be fine if the error would be thrown when using diary's functions, cause then I would simply catch that error in my app. However, because diary tries to access localStorage directly at the top level of the module, this error gets thrown on import of any diary function.

I could probably try/catch the whole file I'm using diary in, but that's not very nice. Ideally diary would delay access to localStorage until it needs it in the reporter, so there are options for consuming code to provide alternatives when localStorage is not available.

First of all, thanks for noticing and mentioning ulog! 🤗

Now for this issue, I wonder if you noticed my other project, anylogger?
Would you consider helping to support it? I would love to have an anylogger-diary adapter 😃

EDIT: I already set one up: anylogger-diary. Would you like to help?

TypeScript is having trouble finding the package types in the latest version of diary.

I created a minimal repro: https://github.com/maxmilton/repro-diary-types

CLI output

> tsc --noEmit

packages/app-api/src/log.ts:3:23 - error TS2307: Cannot find module 'diary' or its corresponding type declarations.

3 import { diary } from 'diary';
                        ~~~~~~~

Versions

• Node: v16.2.0
• Yarn: v1.22.10
• TypeScript: v4.3.2
• Diary: v0.1.2
• OS: Linux max-l 5.12.7-arch1-1 #1 SMP PREEMPT Wed, 26 May 2021 22:03:57 +0000 x86_64 GNU/Linux

On a side note, I'm liking the new API 🥇 . Feels quite good for creating custom reporters.

While #9 is marked as solved, I still have that issue. However, the workaround mentioned there to add the module definition like so:

// global.d.ts

declare module 'diary' {
    export * from 'diary/types/index';
}

works well. So this is not really blocking, but it being a fully typescript library it's a bit weird to have to add these module definitions.

Environment

I am running a bit of a weird setup so this might be the cause

node: v14.16.0
npm: 6.4.11
typescript: 3.9.1-rc

It is also an emberJS project, which has its own rather opaque build system

To more closely mimic the console.log api, it could be nice to allow the following:

const logger = diary("myScope");
const myCoolObject = { foo: "bar" };
logger.log(myCoolObject);

This technically works, but is not allowed by the types. But more importantly, this logs the toString representation of the object, rather than passing it to the default console.log formatter.

I can see the benefit of restricting the api such that there should always be a message, so I would understand if you don't want to go for it. After all, when you are logging anything serious, providing no context is bad practice. But for the sake of uniformity, I like to use the same library for quick debug-logging which I will remove later as for "real logging" which will remain in the production app. It's purely a design decision and is not blocking at all, just thought I'd open the discussion.

Is this feature in relation to a problem?

I want to use an env variable to filter log by level. For example, when I use LOG_LEVEL=error, I dont want to see log in level debug.

Describe the solution you'd like

Provide a setup function like enable, so it does not read env var initially, but user can pass env var to it

Alternative

No response

Additional context

No response

If you have any ideas on how to get this library supported in deno, please help me raise a PR.

Describe the bug

Hey there,

using diary in Frontend environments bundled by webpack, vite or other without defining the process.env variable the library crashes.

I've identified that it tries to access it in the index.ts file when it thinks it's in a node porcess. Actually it should correctly identify that it's getting used in a browser.

Best

Alex

To reproduce

Use diary in a browser project which doesn't use process.env.

Expected behavior

It should not crash - by detecting it's running in a browser instead of a node.js process

Situation

Currently diary before any middleware is processed we check if the currently configured level matches that log event. ie diary.info will not run when the level is set to fatal for instance.

Secondly, if this level matches, we then check that it matches an item in the allow list, eg DEBUG=test:* when diary('test').

When in practice only level should prohibit logs, and rather allow_list should only influence the console output.

Solution

We move the allow list check till after the middlewares are ran to simply control the console output.

This approach should also allow for different styles of outputs, and potentially make the console out fall into a middlware itself—and aid at resolving #4

Hey!

Your logging library looks great. And thanks for doing the Roarr vs Diary benchmark.

It highlighted few performance issues with Roarr.

Please upgrade Roarr to the latest version. :-)

Also, the benchmark is not properly setup. You are currently calling log.child on every test. This is pretty expensive operation and not something you would do in real-world before every log message.

See this as an example of how benchmarks should be initiated:

https://github.com/gajus/roarr/blob/0f6330ed2e290cdf56faf69144834a361c1bf178/test/benchmark.js#L33-L37

Alternatively, you can simply do:

const roarrEvents: any[] = [];

ROARR.write = (message) => {
	roarrEvents.push(message);
};

(async function () {
	await runner({
		roarr() {
			roarr.info('info message');
			return roarrEvents;
		},
	});
})();

The current benchmarks are as follows:

Benchmark
  roarr      x 483,734 ops/sec ±13.25% (71 runs sampled)
  diary      x 395,050 ops/sec ±16.40% (63 runs sampled)

As abit of background, diary has been largely inspired by the logging in "backend" world—such as go or rust. With the idea that you simply run log.debug with some message and move on. And then that message is enhanced with namespace in C# world, or the module in Rust and so on... Or any sort of context that helps a developer know where that came from. This automatic scoping is what makes this beautiful.

Diary was then built to support this "basic" api, feather light runtime and optimal performance; but still lacks this original design goal.

So with this issue I wish to introduce a babel (webpack loader??) to do exactly this. Am envisioning something like;

// input
import { info } from 'diary';

const DoWork = () {
    info('start doing work');
    // work
    info('stop doing work');
}

// output
import { diary } from 'diary';

const _scope_a = diary('npm-package:my-file:DoWork');

const DoWork = () {
    _scope_a.info('start doing work');
    // work
    _scope_a.info('stop doing work');
}

in that when this logs you get automatic scoping of these logs akin to that of go or rust.

There is however some caveats to this, right now the spinng up of diary is expensive as it creates the hooks pipeline, where when authoring this the user may not expect a file with 20fncs, to now have 20 diaries created.

Also what about nested functions?

// output
import { diary } from 'diary';

const _scope_a = diary('npm-package:my-file:DoWork');
const _scope_b = diary('npm-package:my-file:DoWork:DoOtherWork');

const DoWork = () {
    const DoOtherWork = () => {_scope_b.info('doing other work');};
    _scope_a.info('start doing work');
    // work
    DoOtherWork();
    _scope_a.info('stop doing work');
}

should we created a new diary for every scope?

An alternate approach to this could be simply enhance the orignal loggers with some "meta"—leaning on gzip to optmize this. eg:

// output
import { info } from 'diary';

const DoWork = () {
    const DoOtherWork = () => {info('doing other work', '__npm-package:my-file:DoWork:DoOtherWork');};
    _scope_a.info('start doing work', '__npm-package:my-file:DoWork');
    // work
    DoOtherWork();
    _scope_a.info('stop doing work', '__npm-package:my-file:DoWork');
}

with the thinking here that we use the last argument as the "meta"—so that the api for consumers stays consistent. (double underscore denoting our thing for those not using babel).

Pro here is we get the pipeline of hooks optimized so scoping diaries can still happen for those that care.

Also thinking here is to build adapters for React and such—to allow context about component/hook running etc. Maybe even open this api up to allow other constructs to feed context into things. Like xstate. This may be out of scope for this library, but can surely help build the capability.