Amir Blum

Results 96 comments of Amir Blum
trafficstars

> By the way, the handling of `enabled` flag is not done consistent as of now, see #2257 > > I think a single `enable` / `disable` flag is not...

I support imporvments to the `NodeSdk` and `auto-instrumentation-node` as suggested above, in parallel to updating the docs which @JamieDanielson suggested. Most of OTEL end users who are reading the READMEs...

> I usually write dash `-`, and since most parts of the repo & contrib seem to be using that. So I'm in favor of aligning it to dash. 🙂...

> I'm on team dash, but I'm also not sure how important it is to enforce. I'm easy. I was trying to aggregate data from the READMEs of multiple instrumentations...

@david-luna Thank you for the feedback. > > By making this property optional, we are unable to add additional parameters to the constructor function in a clean way, `which is...

Pros options 1: - shorter to write - bit more performant (I think) Pros options 2: - will work correctly for advanced cases if the function is overridden - can...

Also, I support moving this PR out of draft and asking for reviews from the community so we can progress towards merging this.

> What if we remove the concept of async resources entirely? We could simply update the SDK to accept sync resource detectors and/or sync resource objects (all stable components already...

Might be useful to also document it in [the README](https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/opentelemetry-sdk-node) for the SDK package

This version was removed in #243 since the automatic tests failed for this version. I don't know what changes are required, but if you have time to do the research...