Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(otlp-proto): pre-compile proto files #3098

Merged
merged 5 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
}
}
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
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