Skip to content

Commit

Permalink
feat(otlp-proto): pre-compile proto files (#3098)
Browse files Browse the repository at this point in the history
Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
  • Loading branch information
legendecas and vmarchaud authored Jul 20, 2022
1 parent 9ac9976 commit 1e8b896
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 62 deletions.
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ All notable changes to experimental packages in this project will be documented

### :rocket: (Enhancement)

* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
MockedResponse,
} from './traceHelper';
import { CompressionAlgorithm, OTLPExporterNodeConfigBase, OTLPExporterError } from '@opentelemetry/otlp-exporter-base';
import { getExportRequestProto } from '@opentelemetry/otlp-proto-exporter-base';
import { getExportRequestProto, ServiceClientType } from '@opentelemetry/otlp-proto-exporter-base';
import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer';

let fakeRequest: PassThrough;
Expand Down Expand Up @@ -208,8 +208,8 @@ describe('OTLPTraceExporter - node with proto over http', () => {

let buff = Buffer.from('');
fakeRequest.on('end', () => {
const ExportTraceServiceRequestProto = getExportRequestProto();
const data = ExportTraceServiceRequestProto?.decode(buff);
const ExportTraceServiceRequestProto = getExportRequestProto(ServiceClientType.SPANS);
const data = ExportTraceServiceRequestProto.decode(buff);
const json = data?.toJSON() as IExportTraceServiceRequest;
const span1 = json.resourceSpans?.[0].scopeSpans?.[0].spans?.[0];
assert.ok(typeof span1 !== 'undefined', "span doesn't exist");
Expand Down Expand Up @@ -294,8 +294,8 @@ describe('OTLPTraceExporter - node with proto over http', () => {
let buff = Buffer.from('');
fakeRequest.on('end', () => {
const unzippedBuff = zlib.gunzipSync(buff);
const ExportTraceServiceRequestProto = getExportRequestProto();
const data = ExportTraceServiceRequestProto?.decode(unzippedBuff);
const ExportTraceServiceRequestProto = getExportRequestProto(ServiceClientType.SPANS);
const data = ExportTraceServiceRequestProto.decode(unzippedBuff);
const json = data?.toJSON() as IExportTraceServiceRequest;
const span1 = json.resourceSpans?.[0].scopeSpans?.[0].spans?.[0];
assert.ok(typeof span1 !== 'undefined', "span doesn't exist");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { diag } from '@opentelemetry/api';
import { ExportResultCode } from '@opentelemetry/core';
import { getExportRequestProto } from '@opentelemetry/otlp-proto-exporter-base';
import { getExportRequestProto, ServiceClientType } from '@opentelemetry/otlp-proto-exporter-base';
import * as assert from 'assert';
import * as http from 'http';
import * as sinon from 'sinon';
Expand Down Expand Up @@ -236,8 +236,8 @@ describe('OTLPMetricExporter - node with proto over http', () => {
let buff = Buffer.from('');

fakeRequest.on('end', () => {
const ExportTraceServiceRequestProto = getExportRequestProto();
const data = ExportTraceServiceRequestProto?.decode(buff);
const ExportTraceServiceRequestProto = getExportRequestProto(ServiceClientType.METRICS);
const data = ExportTraceServiceRequestProto.decode(buff);
const json = data?.toJSON() as IExportMetricsServiceRequest;

const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0];
Expand Down
2 changes: 2 additions & 0 deletions experimental/packages/otlp-proto-exporter-base/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
src/generated/*
!src/generated/.gitkeep
13 changes: 5 additions & 8 deletions experimental/packages/otlp-proto-exporter-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@
"repository": "open-telemetry/opentelemetry-js",
"scripts": {
"prepublishOnly": "npm run compile",
"compile": "tsc --build",
"compile": "npm run protos && tsc --build",
"clean": "tsc --build --clean",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"postcompile": "npm run submodule && npm run protos:copy",
"protos:copy": "cpx protos/opentelemetry/**/*.* build/protos/opentelemetry",
"protos": "npm run submodule && node scripts/protos.js",
"submodule": "git submodule sync --recursive && git submodule update --init --recursive",
"version": "node ../../../scripts/version-update.js",
"watch": "npm run protos:copy && tsc -w",
"watch": "npm run protos && tsc -w",
"precompile": "lerna run version --scope $(npm pkg get name) --include-dependencies",
"prewatch": "npm run precompile"
},
Expand All @@ -37,7 +36,6 @@
"build/src/**/*.js",
"build/src/**/*.js.map",
"build/src/**/*.d.ts",
"build/protos/**/*.proto",
"doc",
"LICENSE",
"README.md"
Expand All @@ -52,9 +50,9 @@
"@types/node": "14.17.33",
"@types/sinon": "10.0.6",
"codecov": "3.8.3",
"cpx": "1.5.0",
"mocha": "7.2.0",
"nyc": "15.1.0",
"protobufjs": "^6.9.0",
"rimraf": "3.0.2",
"sinon": "12.0.1",
"ts-loader": "8.3.0",
Expand All @@ -67,7 +65,6 @@
"dependencies": {
"@grpc/proto-loader": "^0.6.9",
"@opentelemetry/core": "1.4.0",
"@opentelemetry/otlp-exporter-base": "0.30.0",
"protobufjs": "^6.9.0"
"@opentelemetry/otlp-exporter-base": "0.30.0"
}
}
58 changes: 58 additions & 0 deletions experimental/packages/otlp-proto-exporter-base/scripts/protos.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
'use strict';

const cp = require('child_process');
const path = require('path');

const generatedPath = path.resolve(__dirname, '../src/generated');
const protosPath = path.resolve(__dirname, '../protos');
const protos = [
'opentelemetry/proto/common/v1/common.proto',
'opentelemetry/proto/resource/v1/resource.proto',
'opentelemetry/proto/trace/v1/trace.proto',
'opentelemetry/proto/collector/trace/v1/trace_service.proto',
'opentelemetry/proto/metrics/v1/metrics.proto',
'opentelemetry/proto/collector/metrics/v1/metrics_service.proto',
].map(it => {
return path.join(protosPath, it);
});

function exec(command, argv) {
return new Promise((resolve, reject) => {
const child = cp.spawn(process.execPath, [command, ...argv], {
stdio: ['ignore', 'inherit', 'inherit'],
});
child.on('exit', (code, signal) => {
if (code !== 0) {
reject(new Error(`${command} exited with non-zero code(${code}, ${signal})`));
return;
}
resolve();
})
});
}

function pbts(pbjsOutFile) {
const pbtsPath = path.resolve(__dirname, '../node_modules/.bin/pbts');
const pbtsOptions = [
'-o', path.join(generatedPath, 'root.d.ts'),
]
return exec(pbtsPath, [...pbtsOptions, pbjsOutFile]);
}

async function pbjs(files) {
const pbjsPath = path.resolve(__dirname, '../node_modules/.bin/pbjs');
const outFile = path.join(generatedPath, 'root.js');
const pbjsOptions = [
'-t', 'static-module',
'-w', 'commonjs',
'--null-defaults',
'-o', outFile,
];
await exec(pbjsPath, [...pbjsOptions, ...files]);
return outFile;
}

(async function main() {
const pbjsOut = await pbjs(protos);
await pbts(pbjsOut);
})();
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,6 @@ export abstract class OTLPProtoExporterNodeBase<
promise.then(popPromise, popPromise);
}

override onInit(config: OTLPExporterNodeConfigBase): void {
// defer to next tick and lazy load to avoid loading protobufjs too early
// and making this impossible to be instrumented
setImmediate(() => {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { onInit } = require('./util');
onInit(this, config);
});
}

override send(
objects: ExportItem[],
onSuccess: () => void,
Expand Down
Empty file.
50 changes: 15 additions & 35 deletions experimental/packages/otlp-proto-exporter-base/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,50 +14,29 @@
* limitations under the License.
*/

import * as path from 'path';

import { ServiceClientType } from './types';
import { OTLPProtoExporterNodeBase } from './OTLPProtoExporterNodeBase';
import type { Type } from 'protobufjs';
import * as protobufjs from 'protobufjs';
import {
CompressionAlgorithm,
OTLPExporterError,
OTLPExporterNodeConfigBase,
sendWithHttp
} from '@opentelemetry/otlp-exporter-base';
import type * as protobuf from 'protobufjs';
import * as root from './generated/root';

let ExportRequestProto: Type | undefined;

export function getExportRequestProto(): Type | undefined {
return ExportRequestProto;
export interface ExportRequestType<T, R = T & { toJSON: () => unknown }> {
create(properties?: T): R;
encode(message: T, writer?: protobuf.Writer): protobuf.Writer;
decode(reader: (protobuf.Reader | Uint8Array), length?: number): R;
}

export function onInit<ExportItem, ServiceRequest>(
collector: OTLPProtoExporterNodeBase<ExportItem, ServiceRequest>,
_config: OTLPExporterNodeConfigBase
): void {
const dir = path.resolve(__dirname, '..', 'protos');
const root = new protobufjs.Root();
root.resolvePath = function (origin, target) {
return `${dir}/${target}`;
};
if (collector.getServiceClientType() === ServiceClientType.SPANS) {
const proto = root.loadSync([
'opentelemetry/proto/common/v1/common.proto',
'opentelemetry/proto/resource/v1/resource.proto',
'opentelemetry/proto/trace/v1/trace.proto',
'opentelemetry/proto/collector/trace/v1/trace_service.proto',
]);
ExportRequestProto = proto?.lookupType('ExportTraceServiceRequest');
export function getExportRequestProto<ServiceRequest>(
clientType: ServiceClientType,
): ExportRequestType<ServiceRequest> {
if (clientType === ServiceClientType.SPANS) {
return root.opentelemetry.proto.collector.trace.v1.ExportTraceServiceRequest as unknown as ExportRequestType<ServiceRequest>;
} else {
const proto = root.loadSync([
'opentelemetry/proto/common/v1/common.proto',
'opentelemetry/proto/resource/v1/resource.proto',
'opentelemetry/proto/metrics/v1/metrics.proto',
'opentelemetry/proto/collector/metrics/v1/metrics_service.proto',
]);
ExportRequestProto = proto?.lookupType('ExportMetricsServiceRequest');
return root.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest as unknown as ExportRequestType<ServiceRequest>;
}
}

Expand All @@ -70,9 +49,10 @@ export function send<ExportItem, ServiceRequest>(
): void {
const serviceRequest = collector.convert(objects);

const message = getExportRequestProto()?.create(serviceRequest);
const exportRequestType = getExportRequestProto<ServiceRequest>(collector.getServiceClientType());
const message = exportRequestType.create(serviceRequest);
if (message) {
const body = getExportRequestProto()?.encode(message).finish();
const body = exportRequestType.encode(message).finish();
if (body) {
sendWithHttp(
collector,
Expand Down
2 changes: 2 additions & 0 deletions experimental/packages/otlp-proto-exporter-base/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
{
"extends": "../../../tsconfig.base.json",
"compilerOptions": {
"allowJs": true,
"rootDir": ".",
"outDir": "build"
},
"include": [
"src/**/*.ts",
"src/generated/*.js",
"test/**/*.ts"
],
"references": [
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "OpenTelemetry is a distributed tracing and stats collection framework.",
"scripts": {
"precompile": "lerna run version",
"compile": "tsc --build",
"compile": "lerna run protos && tsc --build",
"prewatch": "npm run precompile",
"watch": "tsc --build --watch",
"clean": "tsc --build --clean",
Expand Down

0 comments on commit 1e8b896

Please sign in to comment.