open-telemetry / opentelemetry-js Goto Github PK
View Code? Open in Web Editor NEWOpenTelemetry JavaScript Client
Home Page: https://opentelemetry.io
License: Apache License 2.0
OpenTelemetry JavaScript Client
Home Page: https://opentelemetry.io
License: Apache License 2.0
Background: OpenCensus Node uses Lerna to organize codebase into multi-package repositories. Lerna give us the ability to build libraries and apps in a single repo without forcing us to publish to npm or other registries. This results in faster code-test-debug cycles by sharing components locally.
https://hackernoon.com/the-highs-and-lows-of-using-lerna-to-manage-your-javascript-projects-ff5c5cd82a99 -> mentioned some highs and lows of using lerna.
@draffensperger shared below article with me (offline).
https://gist.github.com/nolanlawson/457cdb309c9ec5b39f0d420266a9faa4
If possible, provide a recipe for reproducing the error.
$ yarn codecov
Viewing a source file at https://codecov.io/gh/open-telemetry/opentelemetry-js/tree/master/src leads to 404 because it is missing a packages/opentelemetry-<package>/
prefix.
Files are viewable from codecov.io
Can be solved by simply publishing the reports scoped from the root of the repo, instead of at the package level as it is now.
Also this is probably more of an enhancement instead of a bug
I've been working on the OpenCensus Web project, which uses types from OpenCensus Node. However, one of the challenges we ran into is that the @opencensus/core
NPM package that had the core trace/stats model types also imported Node-specific libraries like continuation-local-storage, etc. That made it hard to pull in the types only to make a web-specific implementation of them (I ended up writing a copy script).
I think having a clean package with no dependencies besides TypeScript and maybe proto files would be a nice way to make the Node and Web implementations share an API but be free to diverge in the specifics.
We could also try to have a shared core that is common between Node and Web environments, but even simple things like getting a high-resolution timestamp or generating a secure random number are done differently in Node vs. the browser. There are also different tradeoffs like Node wanting raw performance vs. the browser wanting small code size. So I think separate implementations is OK, but we should share common types.
Originates from #3 (comment)
Follow up of #110
Add a PluginLoader
(or similar) class providing functionality to hook into module loading and apply plugins to enable tracing.
IMO this should get called in automatic tracer constructor to load all default/configured (ex: http
, grpc
, express
, mysql
etc) plugins.
The OpenCensus Node project used Date
for span start/end times (code link). That has the downside of only only having millisecond accuracy. OpenTracing JS uses number
for milliseconds since the epoch (code link), which currently has ~microsecond accuracy - you can confirm that by testing Date.now() + 0.001
vs. Date.now() + 0.0001
.
I think microsecond accuracy is pretty good, but perhaps for some high performance processes you might want higher accuracy, and I think since we are designing this from the ground up, we should ask the question.
Here's an alternative: we use timestamps from the performance.now()
clock, which Node 8+ supports, and which also has broad browser support even from IE 10+.
Those timestamps can have nanosecond accuracy for processes that have been running for less than about 90 days (try running 90*365*24*3600*1000 + 1e-6
to check the accuracy).
We would need to convert those timestamps into a format that the exporter expects in a careful way to preserve the accuracy, see for example [this function]
(https://github.com/census-instrumentation/opencensus-web/blob/82464ed231ad829a7a8a72eab17e95577aa813d0/packages/opencensus-web-core/src/common/time-util.ts#L37) that converts a high-accuracy performance.now
timestamp and a high-accuracy origin time into a nano-second accuracy ISO date string.
If we just want to do number
as millis since the epoch, I'm fine with that too! But wanted to at least open this for discussion. I think use of performance.now
as a clock function would be nice either way since it's one less incompatibility between Node and the browser we have to think about.
This API provides a static global accessor for telemetry objects Tracer
and Meter
(this may be later).
Specs on Obtaining a tracer.
OpenTelemetry-java also has a global OpenTelemetry instance that, among other things, holds a reference to a global tracer: https://github.com/open-telemetry/opentelemetry-java/blob/fb0f5339cd1d665a98996806f45c60c65f47d7b6/api/src/main/java/io/opentelemetry/OpenTelemetry.java#L49-L58
OpenTracing javascript has same kind of code for global tracer object:
https://github.com/opentracing/opentracing-javascript/blob/master/src/global_tracer.ts#L51
Let's make sure our dependencies are up to date.
Setup Codecov(https://codecov.io/) for unit tests, and enforce minimum threshold to >= 80%.
Add separate packages for Context Propagation/Continues Local Storage (CLS). The primary objective of CLS is to implement a transparent context API, that is, we don't need to pass around a ctx
variable everywhere in application code.
In today's (06-19-19) SIG meeting, we have decided to implement generic API, so that it can be used outside OpenTelemetry project.
opentelemetry/context-base
The base implementation that doesn't propagate the context, but handles things like binding.
opentelemetry/context-async-hooks
Implement Async Hooks - async_hooks (since we will be targeting Node 8+).
OpenTracing work on ScopeManager: opentracing/opentracing-javascript#113
OpenCensus work: https://github.com/census-instrumentation/opencensus-node/blob/master/packages/opencensus-core/src/internal/cls-ah.ts
Set/Define guidelines for repository contributors.
I just saw that Event.ts and Sampler.ts are uppercase while we stick to lowercase for the rest.
Do we have a convention for that?
Originates from #6.
Add an automatic tracer (automatically patches well-known modules) with CLS or https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-context-async-hooks package.
OpenCensus implementation: https://github.com/census-instrumentation/opencensus-node/tree/master/packages/opencensus-nodejs-base/src/trace
There is a de-factor standard for a "browser"
field in the package.json
(see package-browser-field-spec) that webpack, Rollup, etc. respect.
It allows us to specify replacement modules for when the package is used in the browser instead of Node. So we could structure things like this:
// src/platform/index.ts
export * from './node'; // Use Node platform by default
// src/platform/node/index.ts
export * from './id'; // Has Node specific `randomSpanId`
// src/platform/browser/index.ts
export * from './id'; // Has browser specific `randomSpanId`
Then in our package.json
for opentelemetry-core
we have this:
// packages/opentelemetry-core/package.json
{
...
"browser": {
"./src/platform/index.ts": "./src/platform/browser/index.ts"
},
...
}
Decide which package manager to use. Both OpenCensuse Node and opentracing-javascript are using npm.
Originates from #2 (comment)
Since it doesn't seem like a special interest group has been formally set up for the node repo, I'd like to propose setting up a SIG similarly to how the dotnet group did theirs. This means setting up a weekly 30 minute sync up, each meeting with a proposed agenda, and a publicly viewable document with meeting notes.
An example of a first meeting's agenda (stolen from dotnet's first meeting):
Thoughts and opinions? What day and time should the meeting take place?
Span
has a private tracer
property, NoRecordSpan
doesn't. I'd prefer to make tracer always available on Span like in OpenTracing. Coming from OpenTracing, I found it challenging not having tracer
available on spans, especially that first I assumed it is and started to use it, then my code broke one NoRecordSpan
(s).
Same as open-telemetry/opentelemetry-python#6
Reference: census-instrumentation/opencensus-node#536
Is your feature request related to a problem? Please describe.
The Apache-2.0 license is basically non-existent in the Node and JavaScript communities. Most projects use MIT and some commercial vendors prefer to use the slightly more strict BSD family of licenses. There are almost no JS projects using Apache-2.0. This could result in issues since most of the community won't know the implications, which could result in confusion at best and in scaring people off at worst.
Describe the solution you'd like
We should go with a more permissive and less invasive license. Basically the most permissive possible given any restrictions from CNCF. I personally propose using MIT which is used by probably over 99% of community-driven Node and JavaScript projects.
Describe alternatives you've considered
Additional context
I don't know why Apache-2.0 was used and maybe there is a good reason. If that's the case, it should be well explained why the choice was to use such a restrictive license.
V8 provides a native way of getting code coverage.
Here's a blog post about it.
Make coverage 2-3x faster than nyc
.
Originates from #6
With this user will have a full control over the instrumentation and span creation. The base package doesn't load Continuation Local Storage (CLS) or any instrumentation plugin by default.
Please answer these questions before submitting a bug report.
Current master branch
10.16
If possible, provide a recipe for reproducing the error.
npm run bootstrap
Successful bootstrap
> lerna bootstrap
lerna notice cli v3.15.0
lerna info Bootstrapping 2 packages
lerna info Symlinking packages and binaries
lerna info lifecycle @opentelemetry/[email protected]~prepare: @opentelemetry/[email protected]
> @opentelemetry/[email protected] prepare /Users/absi/opentelemetry/opentelemetry-js/packages/opentelemetry-core
> yarn run compile:release
yarn run v1.7.0
$ tsc -p tsconfig-release.json
src/common/util/id.ts:17:25 - error TS2307: Cannot find module 'crypto'.
17 import * as crypto from 'crypto';
~~~~~~~~
Found 2 errors.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lerna info lifecycle @opentelemetry/[email protected]~prepare: Failed to exec prepare script
lerna ERR! lifecycle "prepare" errored in "@opentelemetry/core", exiting 1
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] bootstrap: `lerna bootstrap`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] bootstrap script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/absi/.npm/_logs/2019-06-21T08_50_57_808Z-debug.log
Using macos 10.12
Packages:
The opencensus-node only support node 8 and newer. The opentracing/basictracer-javascript does not specify what are the node versions supported in package.json nor documentation. There are implicit versions supported in the Travis file : https://github.com/opentracing/basictracer-javascript/blob/master/.travis.yml#L3
The current status of Node versions:
Need to decide and document supported node versions.
This avoids the caller having to always call span.context()
if they are referencing a span
.
Originates from: #55 (comment) and #31 (comment)
Is your feature request related to a problem? Please describe.
We have already made some tradeoffs around accepting incorrect configuration or data without blowing the SDK up. For example using getCurrentSpan
with NopScopeManager
or passing different format to inject()
. I expect we will log warnings or errors about these edge cases, but I could imagine an extra strict
(default: false) option that would throw instead of logging and could be used for testing.
In OpenCensus I found challenging sometimes to figure out why my traces are broken when I did something wrong.
Describe the solution you'd like
strict
option (default: false) that would throw
instead of logging.
logInconsistency(msg) {
if (this.config.strict) {
throw new Error(msg)
}
this.logger.error(msg);
}
getCurrentSpan(): types.Span {
if (this.scopeManager instanceof NopScopeManager) {
this.logInconsistency('NopScopeManager does not support getCurrentSpan()');
}
return TracerBase.defaultSpan;
}
Describe alternatives you've considered
Just logging.
Originates from #2 (comment)
Is your feature request related to a problem? Please describe.
In JavaScript, it's much easier to use a map-style object to add multiple attributes at once. Having to call setAttribute
multiple times is thus unnecessary and requires more code, especially if copying from an existing object.
Describe the solution you'd like
It should be possible to add multiple attributes using a single call.
For example:
span.setAttributes({
'http.method': 'GET',
'http.path': '/foo/bar'
})
span.setAttributes(existingObject)
The API should also include no-op implementations for some classes, see the java client for details.
Is your feature request related to a problem? Please describe.
As discussed in #33, the license should be added automatically to every file to avoid users forgetting to add it, or add it in an inconsistent way.
Describe the solution you'd like
TSLint can be used for this purpose with the file-header rule. The header should also be reduced to a minimal copyright notice to avoid polluting every file with a large block of license at the top.
Describe alternatives you've considered
Alternatively, we could use a license that doesn't strongly suggest a per-file license. Unfortunately, right now Apache-2.0 is enforced by CNCF so this will need to be discussed outside the scope of this project.
Context Propagation/Continues Local Storage (CLS) is not part of the language, JavaScript or the runtime Node's offering. Although there is some work in Node Core to support distributed tracing use-cases, the solutions today like node-continuation-local-storage come with significant performance overhead and issues, especially with using third-party libraries and a larger amount of async operations like Promises in user-land.
The current practice among APM providers is to hook the Node's module system, monkey patch core modules and bind async primitives via CLS in Node to achieve one line automatic tracing for their customers. This solution is acceptable for most of the Node users, but not for everyone, it's also not compatible with browser environments.
I believe the new OpenTelemetry initiative would give us the opportunity to create a simple synchronous tracer API like OpenTracing and the ability to handle CLS as a separate extension of this API.
(based on current OpenCensus APIs)
Synchronous Tracer APIs:
/**
* Start a new RootSpan to currentRootSpan
* @param options Options for tracer instance
* @returns {Span} A root span
*/
startRootSpan(options: TraceOptions): Span;
/**
* Start a new Span instance to the currentRootSpan
* @param childOf Span
* @param [options] A TraceOptions object to start a root span.
* @returns The new Span instance started
*/
startChildSpan(options?: SpanOptions): Span;
CLS interface to set the context (must be async):
/**
* Sets span to current context
* @param span Span to setup in context.
* @param fn A callback function that runs after context setup.
*/
withSpan<T>(span: Span, fn: () => T): T;
Tracer API combined with CLS:
/**
* Starts a root span and sets it to context.
* @param options Options for tracer instance
* @param fn A callback function to run after starting a root span.
*/
startWithRootSpan<T>(options: TraceOptions, fn: (root: Span) => T): T;
/**
* Start a new Span instance to the currentRootSpan
* @param childOf Span
* @param [options] A TraceOptions object to start a root span.
* @param fn A callback function to run after starting a root span.
*/
startWithChildSpan<T>(options: TraceOptions, fn: (root: Span) => T): T;
An additional benefit of this API is that using withSpan
developer could chain spans in CLS and client RPCs like gRPC would start from the correct contextual child span. OpenCensus today doesn't support context propagation for child spans.
Is your feature request related to a problem? Please describe.
Right now it's not easy to correlate a file with its content. A usual TypeScript project has the name of the file match what it exports. For example: https://github.com/microsoft/TypeScript-Node-Starter/tree/master/src
Describe the solution you'd like
There should be a TSLint rule if possible with the below rules:
Since the @opentelemetry/core
package is intended to have shared code that should be usable in either Node or the browser, we should create browser tests for it.
Initially I suggest unit tests with Karma. Longer term, it would be great to use Sauce Labs to do genuine cross-browser functional testing, likely just for the master
branch.
Came across this, while reviewing #25.
I think we should create a dedicated @opentelemetry org on npm.
opentelemetry-types
would then become @opentelemetry/types
.
As many projects (like Babel) chose this route, this looks like a good practice.
A separate package with only TS interfaces and enums. This is based on #3 and discussion in SIGs meeting. This is also useful if some vendors want to use only types and build their API/Implementations.
It would be nice to split this ask in multiple PRs. Maybe, first PR is to add initial package structure with README, package.json, LICENSE etc (same as https://github.com/open-telemetry/opentelemetry-node/pull/2/files) and incremental PRs to add interfaces and enums.
As discussed on #35 there a ways to create those random ids in a more performant way. Additionally, we should make it browser/server agnostic by factoring out the individual random id generation.
The metrics specification calls for two different types of measures: Long and Double. JavaScript has equivalents for these: BigInt
and Number
respectively, howeverBigInt
is still a fairly new addition to ecmascript specification so it isn't supported by older browsers and seems like it was added to node 10.
Rather than having something like
export interface Measure {
createDoubleMeasurement(value: double): Measurement;
createLongMeasurement(value: long): Measurement;
}
should opentelemetry-node only support number
? The measure interface would look like
export interface Measure {
createMeasurement(value: number): Measurement;
}
@rochdev had this idea in #6 (comment)
I would like us to make sure the core packages are compatible with the browser, and I think it will be easier to have Node + browser instrumentation code all living in the same repo to make it easier to keep everything up to date when refactorings happen.
I think the rename would be good for that purpose and to make it clear we are shooting for the broader JS community and not just Node, though Node will be the primary initial implementation target.
Originates from #39 (Adds CircleCI configuration).
I opted to not set up workflows considering that the repository is still pretty simple and I imagine build/test times will be short for a while. I'll happily spend more time on it when testing parallelization is needed.
This is my initial proposal to kick of the discussion on npm packages for OpenTelemetry Node.
opentelemetry/types
A separate package with only TS interfaces and enums. Originates from #3
opentelemetry or opentelemetry/api or opentelemetry/core
This is the core npm package for OpenTelemetry. It contains the public API which is used by the various plugins. Also contains interfaces for traces
, metrics
, tags
, etc.
Q: What do we want to name the new packages: opentelemetry or opentelemetry/api or opentelemetry/core?
opentelemetry/basic-tracer
With this users will have a full control over instrumentation and span creation. The base package doesn't load Continuation Local Storage (CLS) or any instrumentation plugin by default. Originates from census-instrumentation/opencensus-node#495 (comment)
opentelemetry/context-propagation or opentelemetry/context-cls
Separate package for CLS (Continuation Local Storage). The primary objective of CLS is to implement a transparent context API, that is, we don't need to pass around a ctx
variable everywhere in application code.
Q: What do we want to name the new packages: opentelemetry/context-propagation or opentelemetry/context-cls?
opentelemetry/cls-tracer or opentelemetry/automatic-tracer
Automatic tracer with CLS: opentelemetry/basic-tracer
wrapped with opentelemetry/context-propagation
.
Q: What do we want to name the new packages: opentelemetry/cls-tracer or opentelemetry/automatic-tracer?
opentelemetry/exporter-*
Contains standard trace and stats exporters. For instance, Jaeger, Zipkin, Stackdriver, Instana, Prometheus, oc-agent, z-pages etc.
opentelemetry/instrumentation-*
mongodb, redis, ioredis, express, hapi etc.
opentelemetry/transport-*
gRPC, HTTP/2/s, kafka etc.
opentelemetry/propagation-*
B3, BinaryFormat, TraceContext
opentelemetry/resource-util
It contains a collection of utilities for auto detecting monitored resource when exporting stats, based on the environment where the application is running.
Any thoughts about this?
New update (29th May):
Changes as compared to OC:
api
and sdk (basic-tracer and automatic-tracer)
are separate packagesstats
has been merged into metrics
New Update (30th May)
As previously discussed, TypeScript compilation supports many compilation features to alter code depending on the ECMAScript and environment targets. This is problematic since many different targets must be supported with a single build output. It also makes testing much more difficult since in some cases it may make a specific language construct completely unavailable.
Any feature not related to types should be disabled. If shims are needed, they should be manually imported and used depending on specific conditions.
Based on https://w3c.github.io/trace-context/#tracestate-field, there can be a maximum of 32 list-members
in a list. We have decided to truncate the list instead of throwing exception/error when the defined limit is crossed.
Howdy y'all, I've been combing through the opencensus-node code in preparation of the node merger happening.
In opencensus-node, there's this interesting bit of code in (Span).end that forces all children spans to end when the parent span ends. I don't think this should be default behavior as I've seen many valid situations where the parent ends before the child, particularly in follows from
situations where you have async processes. I do like the debug statement being there, and I like the idea of adding an option to force child spans to end.
Should child span truncation continue to be default behavior?
For opencensus folk, what was the rationale behind it?
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.