Skip to content

Commit

Permalink
Merge branch 'main' into trace-metric-grpc
Browse files Browse the repository at this point in the history
  • Loading branch information
dyladan authored Jul 25, 2022
2 parents 25696c0 + d5cf120 commit 3f9dad2
Show file tree
Hide file tree
Showing 16 changed files with 456 additions and 202 deletions.
57 changes: 51 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ Please also see [GitHub workflow](https://github.com/open-telemetry/community/bl
- [TypeScript](https://www.typescriptlang.org/)
- [lerna](https://github.com/lerna/lerna) to manage dependencies, compilations, and links between packages. Most lerna commands should be run by calling the provided npm scripts.
- [MochaJS](https://mochajs.org/) for tests
- [gts](https://github.com/google/gts)
- [eslint](https://eslint.org/)

Most of the commands needed for development are accessed as [npm scripts](https://docs.npmjs.com/cli/v6/using-npm/scripts). It is recommended that you use the provided npm scripts instead of using `lerna run` in most cases.
Expand Down Expand Up @@ -167,9 +166,16 @@ cd packages/opentelemetry-module-name
npm test
```

To run the unit tests continuously in watch mode while developing, use:

```sh
# Run test in watch mode
npm run tdd
```

### Linting

This project uses a combination of `gts` and `eslint`. Just like tests and compilation, linting can be done for all packages or only a single package.
This project uses `eslint` to lint source code. Just like tests and compilation, linting can be done for all packages or only a single package.

```sh
# Lint all modules
Expand All @@ -191,6 +197,19 @@ cd packages/opentelemetry-module-name
npm run lint:fix
```

### Generating docs

We use [typedoc](https://www.npmjs.com/package/typedoc) to generate the api documentation.

To generate the docs, use:

```sh
# Generate docs in the root 'docs' directory
npm run docs
```

The document will be available under `docs` path.

### Adding a package

To add a new package, copy `packages/template` to your new package directory and modify the `package.json` file to reflect your desired package settings. If the package will not support browser, the `karma.conf` and `tsconifg.esm.json` files may be deleted. If the package will support es5 targets, the reference to `tsconfig.base.json` in `tsconfig.json` should be changed to `tsconfig.es5.json`.
Expand Down Expand Up @@ -226,11 +245,37 @@ After adding the package, run `npm install` from the root of the project. This w

If all of the above requirements are met and there are no unresolved discussions, a pull request may be merged by either a maintainer or an approver.

### Generating API documentation

- `npm run docs` to generate API documentation. Generates the documentation in `packages/opentelemetry-api/docs/out`

### Generating CHANGELOG documentation

- Generate and export your [Github access token](https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token): `export GITHUB_AUTH=<your_token>`
- `npm run changelog` to generate CHANGELOG documentation in your terminal (see [RELEASING.md](RELEASING.md) for more details).

### Platform conditional exports

Universal packages are packages that can be used in both web browsers and
Node.js environment. These packages may be implemented on top of different
platform APIs to achieve the same goal. Like accessing the _global_ reference,
we have different preferred ways to do it:

- In Node.js, we access the _global_ reference with `globalThis` or `global`:

```js
/// packages/opentelemetry-core/src/platform/node/globalThis.ts
export const _globalThis = typeof globalThis === 'object' ? globalThis : global;
```

- In web browser, we access the _global_ reference with the following definition:

```js
/// packages/opentelemetry-core/src/platform/browser/globalThis.ts
export const _globalThis: typeof globalThis =
typeof globalThis === 'object' ? globalThis :
typeof self === 'object' ? self :
typeof window === 'object' ? window :
typeof global === 'object' ? global :
{} as typeof globalThis;
```

Even though the implementation may differ, the exported names must be aligned.
It can be confusing if exported names present in one environment but not in the
others.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
<strong>
<a href="https://github.com/open-telemetry/opentelemetry-js/blob/main/CONTRIBUTING.md">Contributing</a>
&nbsp;&nbsp;&bull;&nbsp;&nbsp;
<a href="https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/development-guide.md">Development Guide</a>
&nbsp;&nbsp;&bull;&nbsp;&nbsp;
<a href="https://github.com/open-telemetry/opentelemetry-js/tree/main/examples">Examples</a>
</strong>
</p>
Expand Down
94 changes: 0 additions & 94 deletions doc/development-guide.md

This file was deleted.

3 changes: 3 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ All notable changes to experimental packages in this project will be documented

### :rocket: (Enhancement)

* feat(metrics-api): use common attributes definitions #3038 @legendecas
* feat(otlp-proto): pre-compile proto files [#3098](https://github.com/open-telemetry/opentelemetry-js/pull/3098) @legendecas

### :bug: (Bug Fix)

* fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc
* fix(exporter-grpc): prevent trace and metrics grpc service clients from interfering with each other [#3100](https://github.com/open-telemetry/opentelemetry-js/pull/3100) @dyladan
* refactor trace grpc exporter to not use the grpc exporter base class
* fix(sdk-metrics-base): fix PeriodicExportingMetricReader keeping Node.js process from exiting
[#3106](https://github.com/open-telemetry/opentelemetry-js/pull/3106) @seemk

### :books: (Refine Doc)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { Context } from '@opentelemetry/api';
import { Context, SpanAttributes, SpanAttributeValue } from '@opentelemetry/api';
import { BatchObservableResult, ObservableResult } from './ObservableResult';

/**
Expand Down Expand Up @@ -82,10 +82,21 @@ export interface Histogram {
record(value: number, attributes?: MetricAttributes, context?: Context): void;
}

// api.SpanAttributes instead of api.Attributes is used here for api package backward compatibility.
/**
* key-value pairs passed by the user.
* Attributes is a map from string to attribute values.
*
* Note: only the own enumerable keys are counted as valid attribute keys.
*/
export type MetricAttributes = SpanAttributes;

// api.SpanAttributeValue instead of api.AttributeValue is used here for api package backward compatibility.
/**
* Attribute values may be any non-nullish primitive value except an object.
*
* null or undefined attribute values are invalid and will result in undefined behavior.
*/
export type MetricAttributes = { [key: string]: string };
export type MetricAttributeValue = SpanAttributeValue;

/**
* The observable callback for Observable instruments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
DataPoint,
Histogram,
} from '@opentelemetry/sdk-metrics-base';
import type { MetricAttributes } from '@opentelemetry/api-metrics';
import type { MetricAttributes, MetricAttributeValue } from '@opentelemetry/api-metrics';
import { hrTimeToMilliseconds } from '@opentelemetry/core';

type PrometheusDataTypeLiteral =
Expand All @@ -38,9 +38,15 @@ function escapeString(str: string) {
return str.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
}

function escapeAttributeValue(str: string) {
/**
* String Attribute values are converted directly to Prometheus attribute values.
* Non-string values are represented as JSON-encoded strings.
*
* `undefined` is converted to an empty string.
*/
function escapeAttributeValue(str: MetricAttributeValue = '') {
if (typeof str !== 'string') {
str = String(str);
str = JSON.stringify(str);
}
return escapeString(str).replace(/"/g, '\\"');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,12 +454,16 @@ describe('PrometheusSerializer', () => {
assert.strictEqual(result, `test_total 1 ${mockedHrTimeMs}\n`);
});

it('should serialize non-string attribute values', async () => {
it('should serialize non-string attribute values in JSON representations', async () => {
const serializer = new PrometheusSerializer();

const result = await testSerializer(serializer, 'test_total', counter => {
counter.add(1, ({
true: true,
false: false,
array: [1, undefined, null, 2],
object: {},
Infinity: Infinity,
NaN: NaN,
null: null,
undefined: undefined,
Expand All @@ -468,7 +472,7 @@ describe('PrometheusSerializer', () => {

assert.strictEqual(
result,
`test_total{object="[object Object]",NaN="NaN",null="null",undefined="undefined"} 1 ${mockedHrTimeMs}\n`
`test_total{true="true",false="false",array="[1,null,null,2]",object="{}",Infinity="null",NaN="null",null="null",undefined=""} 1 ${mockedHrTimeMs}\n`
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
*/

import * as api from '@opentelemetry/api';
import { ExportResultCode, globalErrorHandler } from '@opentelemetry/core';
import {
ExportResultCode,
globalErrorHandler,
unrefTimer
} from '@opentelemetry/core';
import { MetricReader } from './MetricReader';
import { AggregationTemporality } from './AggregationTemporality';
import { InstrumentType } from '../InstrumentDescriptor';
Expand Down Expand Up @@ -100,6 +104,7 @@ export class PeriodicExportingMetricReader extends MetricReader {
globalErrorHandler(err);
}
}, this._exportInterval);
unrefTimer(this._interval);
}

protected async onForceFlush(): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,9 @@ export function hashAttributes(attributes: MetricAttributes): string {
let keys = Object.keys(attributes);
if (keys.length === 0) return '';

// Return a string that is stable on key orders.
keys = keys.sort();
return keys.reduce((result, key) => {
if (result.length > 2) {
result += ',';
}
return (result += key + ':' + attributes[key]);
}, '|#');
return JSON.stringify(keys.map(key => [key, attributes[key]]));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
*/

import * as sinon from 'sinon';
import { callWithTimeout, TimeoutError } from '../src/utils';
import * as assert from 'assert';
import { callWithTimeout, hashAttributes, TimeoutError } from '../src/utils';
import { assertRejects } from './test-utils';
import { MetricAttributes } from '@opentelemetry/api-metrics';

describe('utils', () => {
afterEach(() => {
Expand All @@ -32,4 +34,23 @@ describe('utils', () => {
assertRejects(callWithTimeout(promise, 100), TimeoutError);
});
});

describe('hashAttributes', () => {
it('should hash all types of attribute values', () => {
const cases: [MetricAttributes, string][] = [
[{ 'string': 'bar' }, '[["string","bar"]]'],
[{ 'number': 1 }, '[["number",1]]'],
[{ 'false': false, 'true': true }, '[["false",false],["true",true]]'],
[{ 'arrayOfString': ['foo','bar'] }, '[["arrayOfString",["foo","bar"]]]'],
[{ 'arrayOfNumber': [1,2] }, '[["arrayOfNumber",[1,2]]]'],
[{ 'arrayOfBool': [false,true] }, '[["arrayOfBool",[false,true]]]'],
[{ 'undefined': undefined }, '[["undefined",null]]'],
[{ 'arrayOfHoles': [undefined, null] }, '[["arrayOfHoles",[null,null]]]'],
];

for (const [idx, it] of cases.entries()) {
assert.strictEqual(hashAttributes(it[0]), it[1], `cases[${idx}] failed`);
}
});
});
});
Loading

0 comments on commit 3f9dad2

Please sign in to comment.