Set notifier version to version of bugsnag-atom

[kattrali]

The version number in notifier.version should be the version of the notifier library, rather than the version of the app, which is stored in each event's app.version.
Do you automate updating the version in some way? If so I'll amend the PR to update that as well.

Patched to load the version from package.json

[kattrali]

I'm not quite sure how the test suite should be handled, though I updated the test for the reporter.

[lee-dohm]

Sorry we missed this. To answer your question about LIB_VERSION, the version of the package can be obtained from the package.json here:

https://github.com/atom/exception-reporting/blob/master/package.json#L4

It is updated in an automated fashion.

To ensure that I understand what's intended here, you want to update the notifier version to be the version of the exception-reporting package because the app version is already reported? The app version is always more accurate for our purposes because while a certain version of exception-reporting may be shared across multiple versions of Atom, a particular app version will only have one version of exception-reporting that matches it.

(In any case, I've put this in the queue for review and see about getting other 👀 than mine on it 😀)

[kattrali]

Thanks, @lee-dohm!

To ensure that I understand what's intended here, you want to update the notifier version to be the version of the exception-reporting package because the app version is already reported?

Correct. The notifier properties should be about the exception reporting library itself - it might be the same between versions of atom.

As an example use case, if you were to use the Bugsnag dashboard or API to determine if a new version of the exception-reporting library broke something, or which version introduced a bug, it currently can't be distinguished from the version of atom itself. This probably doesn't make sense as the exception-reporting library has a different release schedule.

The app version is always more accurate for our purposes because while a certain version of exception-reporting may be shared across multiple versions of Atom, a particular app version will only have one version of exception-reporting that matches it.

I think I understand this, but to clarify, notifier.version is used for some debugging purpose by the atom team and is preferred to the app.version property? Would it be useful to have the version of exception-reporting used available as well during triage?

[lee-dohm]

Each version of Atom uses a specific version of each package that is included in the application. For example, you can find exception-reporting here:

https://github.com/atom/atom/blob/a084882cb130fa6f7ffe51a88ebd140292bf4f03/package.json#L91

So while the exception-reporting package has a different release schedule, if we have the Atom version, we can always backtrack to the specific exception-reporting package version if necessary.

Personally, I'm leery of changing the meaning of metadata without changing the name of it because it means that one can't compare data across the breaking point. There is the data set before the change and the data set after the change, but you can't compare the two directly (without excluding the item that was changed from the analysis).

☝️ is my knee-jerk reaction from working on metrics tracking before. I'm not familiar enough with how we use the data to say if this is a valid concern yet. Just trying to clarify 😀

[kattrali]

So while the exception-reporting package has a different release schedule, if we have the Atom version, we can always backtrack to the specific exception-reporting package version if necessary.

Gotcha, I think we thought of this library two different ways, one as an immutable part of an atom release and on the other hand as a replaceable component, where someone could test or use a different version of exception-reporting.

Personally, I'm leery of changing the meaning of metadata without changing the name of it because it means that one can't compare data across the breaking point. There is the data set before the change and the data set after the change, but you can't compare the two directly (without excluding the item that was changed from the analysis).

That makes sense. To be clear, this is not changing the app version which you see in Bugsnag. Unless there is already an integration in place using notifier.version directly (which would have to be using the API), the net effect of changing this is the possibility of Bugsnag tracking the error rate from a particular notifier library and assisting with problems. There were supposedly 200+ versions of the exception-reporting library in use in the last month or so (one for every development version of atom which has reported an error, i.e. "1.6.0-dev-52cab67"), which led me to believe there was a problem with the Atom integration which I should investigate. This is what led me to make the change.

[lee-dohm]

@kattrali Cool, thank you for taking the time to explain it! I'll see about getting one of the other devs that is perhaps more familiar with how we view the data to take a peek. Thanks so much for following up 🙇

[kattrali]

Thank you for taking a look!

[thedaniel]

Looks good to me, specs are green, merging. Thanks for this!

[thedaniel]

This change is in exception-reporting v0.38.0, and will be in atom 1.8 beta. It will be some weeks before the change is reflected in stable.

[kattrali]

Thanks all!