use of eval is strongly discouraged

[lnmp4000]

Bundling with rollupjs generated the following error because of protobufjs usage.

(!) Use of eval is strongly discouraged
https://rollupjs.org/troubleshooting/#avoiding-eval
../node_modules/@protobufjs/inquire/index.js
10: function inquire(moduleName) {
11:     try {
12:         var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval
                      ^
13:         if (mod && (mod.length || Object.keys(mod).length))
14:             return mod;

This is included via the following tree:

  └─┬ @opentelemetry/exporter-trace-otlp-http@0.53.0
    └─┬ @opentelemetry/otlp-transformer@0.53.0
      └── protobufjs@7.4.0

I don't understand why this is necessary, but it could potentially cause issues for code bundled via rollup.

https://rollupjs.org/troubleshooting/#avoiding-eval

Also see https://github.com/protobufjs/protobuf.js/issues/1754

[pichlermarc]

Hi @lnmp4000, thanks for reaching out. I was under the impression that this would actually get tree-shaken out for users of the @opentelemetry/exporter-trace-otlp-http package - but I'm not very familar with rollup so I'd be somewhat time consuming to come up with a repro for this. Maybe it's generating that warning before it's tree-shaken out?

Would you mind providing a small repro so I can investigate? :thinking: Is it an error or a warning that's generated by rollup?

Regardless of tree-shaking, we still use this in @opentelemetry/exporter-trace-otlp-proto and @opentelemetry/exporter-logs-otlp-proto and looking at the the issue you linked, my preferred solution would be to fix this upstream in the protobufjs repo as this seems to affect quite a few people...

[pichlermarc]

Would you mind providing a small repro so I can investigate? 🤔

Nevermind, I took some time to reproduce this :slightly_smiling_face: https://github.com/pichlermarc/repro-4987

Is it an error or a warning that's generated by rollup?

Looking at my reproducer it looks like the following is happening:

• rollup sees the code before tree-shaking it, generates a warning
• code eventually gets tree-shaken out and eval does not end up in the final bundle

Therefore code using @opentelemetry/exporter-trace-otlp-http does not violate CSPs by using eval, but using @opentelemetry/exporter-trace-otlp-proto does, as the eval call will eventually end up in the final bundle.

There's a few ways we can partially/fully fix this:

• Quick partial fix: @opentelemetry/otlp-transformer can have multiple entrypoints, one for json and one for protobuf serialization
  • this will get rid of the log message generated by rollup when using @opentelemetry/exporter-trace-otlp-http
  • it will not fix the underlying issue for @opentelemetry/exporter-trace-otlp-proto
• we can re-write protobuf serialization from scratch, not using generated code, and not using @protobufjs/inquire
  • this will be a lot of work, given that we plan to change the structure of the internal data-model later, it might cause a lot of unnecessary churn and maintenance effort, so I'd like to avoid this where possible :slightly_frowning_face:
  • this will fix both the rollup warning log for @opentelemetry/exporter-trace-otlp-http and the violation of CSPs by @opentelemetry/exporter-trace-otlp-proto
• we can fix this in @protobufjs/inquire
  • this will fix both the rollup warning log for @opentelemetry/exporter-trace-otlp-http and the violation of CSPs by @opentelemetry/exporter-trace-otlp-proto
  • this will fix it for everyone that consumes the @protobufjs/inquire either directly or transitively.
  • this is the :sparkles: ideal solution :sparkles: as it fixes it not just for OTel JS but everyone that uses protobufjs

[lnmp4000]

@pichlermarc Thanks so much for looking at this. In my case, based on your research, It seems I can just ignore this warning for now as the eval code will not be in the final bundle.

[pichlermarc]

@pichlermarc Thanks so much for looking at this. In my case, based on your research, It seems I can just ignore this warning for now as the eval code will not be in the final bundle.

Yes, exactly. :slightly_smiling_face:

If you don't mind, I'll spin off two bugs from this issue to clarify the situation:

• False-positive eval()-use warning for OTLP JSON exporters when using rollup (priority:p4, reason: logspam, misleading log)
• OTLP Protobuf exporters violate CSP by using eval() (priority:p1 reason: causes problems in end-user apps)