Tracing on JS

[vasilmkd]

Decision time.

#2207 is starting to look like a dumping ground for new ideas and the scope seems to be changing every 5 minutes. This is really hard for reviewing, and I personally want to get tracing on JS right.

I would like this discussion to be a list of points that we can achieve consensus on, and then split each point into a separate, focused PRs.

It seems that we need a firm decision on the following things:

1. Source maps
   IMO, source maps are the way to get usable stack traces, especially when we're talking fullOptJS. I'm of the opinion that we should include this in our official recommendation, with the caveat that there is a slight set up curve before hand (npm install and all). On the other hand, this sentence is copy pasted from the official Scala.js website in the Basic Tutorial:

Source maps in Node.js: To get your stack traces resolved on Node.js, you will have to install the source-map-support package.

If we do make the official recommendation ourselves, this steers us in the direction of only testing the node CI with --enable-source-maps. If we don't make this recommendation, I'm honestly not sure what to do, because the different permutations of options for node are getting out of hand.

2. Configure the stack tracing mode using environment variables on node. I think this is self explanatory. I would like to not allow easy access to the mutable stack tracing mode. This probably means that browser users have no "easy" way of changing the stack tracing mode. IMO, cached mode is a sane default. This leads me to the next point.

3. Have an escape hatch in the form of a dynamic type containing a setTracingMode method, similar to what we have for resetGlobal on Scala.js. This is a workaround for browser users who care enough about changing the default stack tracing mode.

4. However, (and I'm a bit ill-informed here), is there good source map support for Scala.js in the browser? Personally I'm not a huge fan of how stack traces look on Firefox (you can see them in the PR). Are browsers even considered a supported execution environment for Scala.js? I feel like Node is the officially supported choice, but what about the browsers.

Feel free to add any points that you think I have missed in summarizing.

cc @armanbilge @djspiewak

[armanbilge]

Thanks for writing this all up!

1. 💯 to sourcemaps.
2. I think we should do this with process.env not just for Node.js, it's an oft-reused idiom.
3. "This is a workaround for browser users who care enough about changing the default stack tracing mode ..." and absolutely hands down refuse to use any sort of bundler post-processing tool and/or hack a process.env global variable into their environment. Like, come on :P BTW, the reason to avoid the escape hatch is that if we can inline process.env then it has a much better chance of being cleanly optimized.
4. So source maps are not a Scala.js thing, they are a pretty standard JS thing. So, yes, source maps are well-supported in the browser. Furthermore, I'd say browsers very much are the original intended target for Scala.js, the homepage literally says "A safer way to build robust front-end web applications!" :)

[armanbilge]

Oh, and maybe a last topic:

5. JS benchmarking. This will be a bit of a :yak:, I took a look at scalajs-benchmark as well as a few other stuff on npm but I think scalajs-benchmark is the best choice, takes its cues from JMH. The :yak: however is it doesn't appear to support profiling an async function atm, so that would have to come first.

[armanbilge]

with the caveat that there is a slight set up curve before hand (npm install and all)

Theoretically we could declare source-map-support a JS dependency of CE3 so it gets installed automatically for downstream dependents (so long as they uses Scala.js bundler). Not sure I'm on board with this, just throwing it out there.

[djspiewak]

Regarding source maps…

I think we shouldn't require them. What we're doing is no worse than the existing ScalaJS stack traces, and is fully compatible (to the best of my understanding) with the the standard approaches to mitigating that problem. In other words, we aren't making anything worse, and we complement the solutions which make everything better. Thus, we should follow ScalaJS's approach and not make the maps mandatory. Though, I would not be averse to mentioning them in the documentation.

Regarding configuration, I don't really have much of a strong opinion on this beyond that I think it should follow any standard-ish JavaScript idioms for doing so. Equally importantly, tracing should be configurable from outside of a ScalaJS environment (e.g. process.env, or a global, or something like that. If we require tracing configuration within ScalaJS then we basically defeat the purpose of IOApp and similar.

[armanbilge]

👍 to optional source maps, although I still think we should strongly recommend them when discussing tracing. For the record nothing in the current tracing implementation requires source maps, although without them enabled for fullOptJS we can't do the run-loop cleanup etc. to the trace. So in a sense, we don't fully support tracing in that particular situation.

If we require tracing configuration within ScalaJS then we basically defeat the purpose of IOApp and similar.

Agree, and also we lose the ability for optimizations to be applied when minifying/bundling JS, etc. I'm very pro process.env, it's an oft-reused idiom in JS world. The question is, how much of a back-door should we leave for those who need/want something different? They can always make their own process.env global, or we can provide more explicit support for this usecase.

[djspiewak]

Continuing this discussion a bit…

There's still one open question, which is whether or not we even should trace under fullOptJS. I'm not sure what users are expecting there. Obviously fullOpt without source maps should not be traced, but fullOpt with source maps is also an open question. If we decide that users simply don't expect the ability to introspect their code or any exceptions which are produced by their code in fullOpt mode, then we can detect fullOpt and trigger dead code elimination on all of the tracing stuff in that case.

Realistically, this kind of comes down to ScalaJS user expectations, and it's something I don't have a good feel for. @sjrd can you perhaps weigh in on this point?

[armanbilge]

I think another argument to DCE tracing in fullOptJS is that currently even with tracing disabled there is still a performance penalty. See my gripes in #2207 (comment). IIUC JVM avoids this penalty by setting final static fields for tracing mode based on properties configured at runtime, which enables branches to be appropriately optimized.

[sjrd]

Is tracing a debugging feature, or a production feature? If it is only a debugging feature, it would definitely make sense to get rid of it in fullOptJS (to be more precise, when scala.scalajs.LinkingInfo.productionMode is true). If it's a production feature, i.e., something where we expect the developers to monitor on a running, production deployment, then it should stay in fullOpt.

[djspiewak]

It's kind of both. Tracing is what makes exceptions useful by replacing the elements of the stack trace which come from the internals of the Cats Effect runtime (which is asynchronous and trampolined and thus lacks any meaningful stack context) with sequential references to your application's call sites. There are other benefits as well, but that's the main one. You can see a quick JVM example here: https://typelevel.org/cats-effect/docs/tracing#enhanced-exceptions

JVM users generally have this enabled in production for the same reason that pretty much everyone compiles with source tables and debug symbols enabled: exceptions happen in production and it's important to be able to track them down. fullOptJS though already effectively eliminates debug symbols, so it's a slightly weird situation to compare.