chore: protobufjs load fix & definitions update script
#70
Conversation
| @@ -38,6 +38,9 @@ const statusCodeEnumFromVal = { | |||
| * to a value for converting 'attributes' to a simpler object, e.g.: | |||
| * { 'telemetry.sdk.version': '1.19.0', | |||
| * 'process.pid': 19667 } | |||
| * | |||
| * @param {import('./types-proto').opentelemetry.proto.common.v1.IAnyValue} v | |||
There was a problem hiding this comment.
note for reviewer: this is a sample of usage of the new types being generated. I've tried to make an alias like
/** @typedef {import('./types-proto').opentelemetry.proto.common} OTelCommon */but JsDoc seems to not be able to do that with namespaces
| const prefix = resolve(__dirname, '..'); | ||
| const paths = [ | ||
| '/opentelemetry/proto/common/v1/common.proto', |
There was a problem hiding this comment.
note for reviewer: protobufjs tried to load these files since they were in the import graph but it was failing silently. That's why having it here made possible for this to work.
Now that the loading process is fixed it is not necessary anymore
| @@ -1,5 +1,3 @@ | |||
| import type EventEmitter from 'events'; | |||
There was a problem hiding this comment.
left over from a previous PR
| 3. `metrics` package contains the Metrics Service protos. | ||
| 4. `logs` package contains the Logs Service protos. | ||
|
|
||
| ### NOTE from Elastic Observability |
There was a problem hiding this comment.
note fort reviewer: this file was not included the 1st time we copied the files. Now we're keeping it and adding a note on the version they files belong to
| @@ -55,9 +55,6 @@ message ResourceLogs { | |||
| // A list of ScopeLogs that originate from a resource. | |||
| repeated ScopeLogs scope_logs = 2; | |||
|
|
|||
| // The Schema URL, if known. This is the identifier of the Schema that the resource data | |||
There was a problem hiding this comment.
note for reviewer: the script did remove changed that were done after the v0.20.0 tag
|
we are starting to have spurious tests with the following error we need to check if |
scripts/update-protos.js
Outdated
| const tempPath = tmpdir(); | ||
| const checkoutPath = `${tempPath}/${REPO_NAME}`; |
There was a problem hiding this comment.
nit: Might be preferable to use fs.mkdtemp() for a guaranteed unique temp dir. Then won't need to the if (existsSync(checkoutPath)) { rmSync(checkoutPath, ... below.
I opened #73 for this. I've hit it a couple times. |
Release of
protobufjs@7.2.6now throws an exception if a.protofile imports another and its not found in the filesystem relative to the file doing the import. This makes ourmockotlpservercrash since opentelemetry proto files have absolute imports. Ref: protobufjs/protobuf.js#1971This PR includes
protobufjsprotofiles synced with the ones inopentelemetry-jsrepo (current files were a bit younger)Script usage:
The result will be
packages/mockotlpserver/opentelemetryat the specified version in the scriptpackages/mockotlpserver/opentelemetry/proto/collector/README.mdfile updated with a note specifying which version was downloaded.packages/mockotlpserver/lib/types-proto.d.tsis updated with the latest changes