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

Grpc OpenTelemetry Collector Exporter not showing traces with webpack #2786

Closed
itobby opened this issue Feb 14, 2022 · 18 comments · Fixed by #3705
Closed

Grpc OpenTelemetry Collector Exporter not showing traces with webpack #2786

itobby opened this issue Feb 14, 2022 · 18 comments · Fixed by #3705
Assignees

Comments

@itobby
Copy link

itobby commented Feb 14, 2022

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

    "@opentelemetry/core": "^1.0.1",
    "@opentelemetry/api": "^1.0.4",
    "@opentelemetry/exporter-trace-otlp-grpc": "^0.27.0",
    "@opentelemetry/resources": "1.0.1",
    "@opentelemetry/sdk-metrics-base": "^0.27.0",
    "@opentelemetry/sdk-trace-base": "^1.0.1",
    "@opentelemetry/semantic-conventions": "1.0.1"

What version of Node are you using?

v16.13.2

Please provide the code you used to setup the OpenTelemetry SDK

Converted the sample app to ts

// tracing.ts
import opentelemetry from "@opentelemetry/api";
import { BasicTracerProvider, ConsoleSpanExporter, SimpleSpanProcessor } from "@opentelemetry/sdk-trace-base";
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-grpc";
import { SemanticResourceAttributes } from "@opentelemetry/semantic-conventions";
import { Resource } from "@opentelemetry/resources";

const exporter = new OTLPTraceExporter();

const provider = new BasicTracerProvider({
    resource: new Resource({
        [SemanticResourceAttributes.SERVICE_NAME]: "basic-service",
    }),
});
provider.addSpanProcessor(new SimpleSpanProcessor(exporter));
provider.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter()));
provider.register();

const tracer = opentelemetry.trace.getTracer("example-otlp-exporter-node");

// Create a span. A span must be closed.
const parentSpan = tracer.startSpan("main");
for (let i = 0; i < 10; i += 1) {
    doWork(parentSpan);
}
// Be sure to end the span.
parentSpan.end();

// give some time before it is closed
setTimeout(() => {
    // flush and close the connection.
    exporter.shutdown();
}, 2000);

function doWork(parent: any) {
    // Start another span. In this example, the main method already started a
    // span, so that'll be the parent span, and this will be a child span.
    const ctx = opentelemetry.trace.setSpan(opentelemetry.context.active(), parent);
    const span = tracer.startSpan("doWork", undefined, ctx);

    // simulate some random work.
    for (let i = 0; i <= Math.floor(Math.random() * 40000000); i += 1) {
        // empty
    }
    // Set attributes to the span.
    span.setAttribute("key", "value");

    // Annotate our span to capture metadata about our operation
    span.addEvent("invoking doWork");

    // end span
    span.end();
}

What did you do?

Added a webpack file, with the following content, and tried running the tracing on the dist/index.js file.

const path = require("path");

module.exports = {
    entry: path.resolve(__dirname, "app.js"),
    target: "node",
    output: {
        path: path.resolve(__dirname, "dist"),
        filename: "index.js",
    },
    module: {
        rules: [
            {
                test: /.(ts|js)$/,
                loader: "ts-loader",
                exclude: /node_modules/,
            },
            { test: /\.node$/, loader: "node-loader" },
        ],
    },
};

What did you expect to see?

I expected to see the tracing from both the Otel Exporter and Console Exporter.

What did you see instead?

The Otel Exporter traces are not showing in localhost:4317, only the console exporter displays in console.

Additional context

This warning is always displayed -
opentelemetry/proto/collector/trace/v1/trace_service.proto not found in any of the include paths js-app\protos
I tried moving the protos folder into the path the warning displays... solves nothing still.

The traces display fine without webpack.

@itobby itobby added the bug Something isn't working label Feb 14, 2022
@itobby itobby changed the title Grpc OpenTelemetry Collector Exporter not showing traces Grpc OpenTelemetry Collector Exporter not showing traces with webpack Feb 14, 2022
@dyladan
Copy link
Member

dyladan commented Feb 14, 2022

the exporter loads the proto files async when it is initialized. Most likely the proto files are not included in the webpack bundle. Your module rules only seem to pick up .ts, .js, and .node files. The proto files are .proto.

@itobby
Copy link
Author

itobby commented Feb 15, 2022

@dyladan Would you be able to give an example on this?
I have tried a few things to no avail. Including copying the physical proto files.
Despite seeing those protos file, the traces still don't show up with webpack.

@dyladan
Copy link
Member

dyladan commented Feb 15, 2022

Unfortunately I'm not a webpack expert and I don't know immediately how to do this. I would search how to bundle proto files in webpack or possibly use something like file-loader to bundle any non-js files.

@markandrus
Copy link

I also ran into this issue when using esbuild to bundle my application for AWS Lambda. The problem for me was this line

const includeDirs = [path.resolve(__dirname, '..', 'protos')];

in conjunction with these lines

getServiceProtoPath(): string {
return 'opentelemetry/proto/collector/trace/v1/trace_service.proto';
}

I took care to copy the opentelemetry/proto/collector/trace/v1/ directory into the app directory which would ultimately execute inside of my Lambda. Then, I bundled my code. The result is

app
├── index.js
└── protos
    └── opentelemetry
        └── proto
            ├── collector
            │   ├── README.md
            │   ├── logs
            │   │   └── v1
            │   │       └── logs_service.proto
            │   ├── metrics
            │   │   └── v1
            │   │       └── metrics_service.proto
            │   └── trace
            │       └── v1
            │           └── trace_service.proto
            ├── common
            │   └── v1
            │       └── common.proto
            ├── logs
            │   └── v1
            │       └── logs.proto
            ├── metrics
            │   ├── experimental
            │   │   └── configservice.proto
            │   └── v1
            │       └── metrics.proto
            ├── resource
            │   └── v1
            │       └── resource.proto
            └── trace
                └── v1
                    ├── trace.proto
                    └── trace_config.proto

However, includeDirs looks for app/../protos instead of app/protos. I had to hack around this in my build script by doing the following:

sed 's/const includeDirs = \[path.resolve(__dirname, "\.\.", "protos")\];/const includeDirs = [path.resolve(__dirname, "protos")];/' <app/index.js >app/index2.js
cp index2.js index.js

Ultimately it worked… but could it be easier?

I don't know how the .proto files are used, but would it be possible to pre-compile them? Or otherwise not need to depend on them at runtime? Alternatively, could this library allow bundle users to explicitly pass the path to .proto files (in the resulting bundle) at runtime?

@dyladan
Copy link
Member

dyladan commented Apr 6, 2022

It is possible to precompile .proto files into their json representations or to static JS but there are other considerations such as bundle size and performance impact there. The proto files are loaded by protobufjs to define the serialization format.

@AWare
Copy link

AWare commented Apr 21, 2022

We've just hit this problem using opentelemetry with esbuild, it would be really helpful if the protos directory could be passed in as a parameter, so that we don't have to artificially hoist them like this.

@pkanal
Copy link
Contributor

pkanal commented Jun 21, 2022

We've run into this problem while using any transpiler or bundler which includes Webpack, ESBuild and TypeScript. A workaround I found that works is configuring your bundler to copy the proto files out of @opentelemetry/otlp-grpc-exporter-base into the correct path (I think this would also be the case for @opentelemetry/otlp-proto-exporter-base. It's really similar to the solution described above, but uses ESBuild / Webpack / TS config to do the work.

For the package to find the proto files in the right spot, the output build directory and the /protos directory have to be at the same level.

ESBuild Config:

const { copy } = require('esbuild-plugin-copy')

require('esbuild').build({
  ... other config ...
  plugins: [
    copy({
      // this is equal to process.cwd(), which means we use cwd path as base path to resolve `to` path
      // if not specified, this plugin uses ESBuild.build outdir/outfile options as base path.
      resolveFrom: 'cwd',
      assets: {
        from: ['./node_modules/@opentelemetry/otlp-grpc-exporter-base/build/protos/**/*'],
        to: ['./protos'],
        keepStructure: true
      },
    }),
  ],
}).catch(() => process.exit(1));

Webpack config:

const CopyPlugin = require("copy-webpack-plugin");

module.exports = {
  plugins: [
    new CopyPlugin({
      patterns: [
        { from: "./node_modules/@opentelemetry/otlp-grpc-exporter-base/build/protos/**/*", to: "./protos" }
      ],
    }),
  ],
};

TypeScript config currently does not have a way to specify copying extra files you may need for your build, so in this case you would have to use a postbuild script to copy the proto files to the correct location.

It would be really helpful to see a solution that doesn't require special build config for bundlers, especially since Node + TypeScript is a very common stack.

@dyladan
Copy link
Member

dyladan commented Jun 21, 2022

One possibility we considered is instead of using .proto files we could check in code generated based on them but nobody has had the time to work on that. @pichlermarc was doing a lot of work factoring out the transformation code that might make that quite a bit easier of a project now if anyone is interested in taking it on.

@dyladan dyladan added feature-request and removed bug Something isn't working labels Jun 29, 2022
@dyladan
Copy link
Member

dyladan commented Jun 29, 2022

I think this is not a bug because it is known the gRPC doesn't work in webpacked environments because of these proto files. There are workarounds like using a webpack plugin to copy the proto files. I'm changing this to a feature request as I view the request "please make the exporter work out of the box with webpack without configuration"

@AWare
Copy link

AWare commented Jul 5, 2022

I disagree that this is not a bug. Bundling is fairly common, and expecting libraries to support bundling is not entirely unreasonable.

The request is not that it work out of the box with webpack without configuration, but that it work with bundlers in general with a clearly documented workaround.

Such a workaround could be that

const includeDirs = [path.resolve(__dirname, '..', 'protos')];
also include path.resolve(__dirname, 'protos') and the documentation said that to work with a bundler, these files must be present- and give a location to get them from.

The workaround we have used is to mark this entire module as external in esbuild and include a node_modules directory with this package in alongside our bundle.

@dyladan
Copy link
Member

dyladan commented Jul 6, 2022

We can discuss this in SIG today

@AWare
Copy link

AWare commented Jul 13, 2022

@dyladan thanks for discussing this, what was the outcome of the discussion?

@dyladan
Copy link
Member

dyladan commented Jul 13, 2022

We agreed this is not a bug. I'm currently reworking the exporters to use generated code instead of loading .proto files so that should fix the webpack issue.

@AWare
Copy link

AWare commented Jul 13, 2022

thanks 👍

@matdehaast
Copy link

@dyladan any news on this?

@dyladan
Copy link
Member

dyladan commented Oct 6, 2022

Not really. The protobuf http exporters now have generated code so they shouldn't have this issue but the gRPC exporters have not gotten the same treatment.

@shuhrat
Copy link

shuhrat commented Feb 17, 2023

👋 Is there any updates on this issue?

@pichlermarc
Copy link
Member

@shuhrat I just opened a PR that should take care of this #3705 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants