search

aandrewww / pino-sentry

🚥 Load pino logs into Sentry
MIT License
61 stars 29 forks source link

Consider changing stackAttributeKey default to 'err.stack' #48

Open segevfiner opened 2 years ago

segevfiner commented 2 years ago

It is currently set to 'stack': https://github.com/aandrewww/pino-sentry/blob/ba2f56bc1fc21e0d1dc331fa71ae618b23562b9f/src/transport.ts#L68

But I think Pino mostly stores stack traces from errors under 'err.stack'.

glensc commented 2 years ago

@segevfiner since we're at semver's initial development (0.x.y), can make this breaking change.

segevfiner commented 2 years ago

Won't you just increment the minor for this? Initial development/version 0, doesn't have as strong compatibility guarantees as something version 1+

aandrewww commented 2 years ago

Someday we will increase the major version :)

glensc commented 2 years ago

as i undestand, with 0.x series any minor version bump is considered "major", bumping patch version is still a patch version update (updated with yarn upgrade).

so, if you have ^0.8 in your deps, yarn upgrade will update 0.8.1 to 0.8.2 but not to 0.9.0

segevfiner commented 2 years ago

Yeah. That's the idea. But it's always a good idea to verify in npm docs, since this is often confusing.

aandrewww commented 2 years ago

@glensc as I understand, it depends on specifying version range (^,~, etc) https://docs.npmjs.com/cli/v8/configuring-npm/package-json#dependencies

glensc commented 2 years ago

@segevfiner I just tested the ^ behavior for 0.x versions yesterday.

glensc commented 2 years ago

@aandrewww i'm talking about semver specifier, the ^, if you use something else, like ~ or >= then that's not semver.

glensc commented 2 years ago

@segevfiner just submit the pr if you want this in.