chore(deps): bump protobufjs & tape patch versions. Fix load of .proto files
#66
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,29 +2,51 @@ | |
| * Some utilities for working with the OpenTelemetry proto files. | ||
| */ | ||
|
|
||
| const {resolve} = require('path'); | ||
|
|
||
| const {Root} = require('protobufjs'); | ||
|
|
||
| // TODO: for now `proto` files are copied from | ||
| // https://github.com/open-telemetry/opentelemetry-proto/releases/tag/v1.0.0 | ||
| // but maybe its better to have a submodule like otel-js does | ||
| const prefix = resolve(__dirname, '..'); | ||
| const paths = [ | ||
| '/opentelemetry/proto/common/v1/common.proto', | ||
| '/opentelemetry/proto/resource/v1/resource.proto', | ||
| '/opentelemetry/proto/logs/v1/logs.proto', | ||
| '/opentelemetry/proto/metrics/v1/metrics.proto', | ||
| '/opentelemetry/proto/trace/v1/trace.proto', | ||
| '/opentelemetry/proto/collector/logs/v1/logs_service.proto', | ||
| '/opentelemetry/proto/collector/metrics/v1/metrics_service.proto', | ||
| '/opentelemetry/proto/collector/trace/v1/trace_service.proto', | ||
| ]; | ||
|
|
||
| // Craete a new Root so we can patch it | ||
| const root = new Root(); | ||
|
|
||
| // This function is patched because the Root class does not have any | ||
| // referece of which is the root path of the proto files. Instead it | ||
| // takes the folder of current file being processed as the root path | ||
| // resulting in duplicated subpaths | ||
| // Example: resource.proto file importing common.proto results in | ||
| // /Users/.../mockotlpserver/opentelemetry/proto/resource/v1/opentelemetry/proto/common/v1/common.proto | ||
| root.resolvePath = function patchResolvePath(filename) { | ||
| let path = Root.prototype.resolvePath.apply(root, arguments); | ||
| if (filename) { | ||
| const folder = resolve(filename, '..'); | ||
| path = prefix + path.replace(folder, ''); | ||
| } | ||
| return path; | ||
| }; | ||
|
There was a problem hiding this comment. Nice. Was the intent to move away from this hack eventually once we have a process to build/commit a proto.json and load that instead? There was a problem hiding this comment. yes! I think as part of the process of updating the There was a problem hiding this comment. Would it possibly work to add a mostly-empty root.proto file at the top-level dir, load that first, then use its returned |
||
|
|
||
| // Load the files at once | ||
| root.loadSync(paths.map((p) => `${prefix}${p}`)); | ||
|
|
||
| /** | ||
| * Return `any` for now itherwise we get type errors when using | ||
| * `root.lookupType(...)` in `normalize.js | ||
| * @returns {any} | ||
| */ | ||
| function getProtoRoot() { | ||
| return root; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,6 @@ | |
| "express": "^4.18.2", | ||
| "module-details-from-path": "^1.0.3", | ||
| "semver": "^7.6.0", | ||
| "tape": "^5.7.4" | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we really using v1.0.0?
I thought it looked like opentelemetry-js.git was using a pre-1.0.0 tag version of the protos. I've no idea if it matters, however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be compatible but I'll sync up in a follow up PR