[http-plugin] ignoreIncomingPaths type error in TS

[naseemkullah]

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

v0.2.0

What version of Node are you using?

TS

What did you do?

added ignoreIncomingPaths: [/\/healthz/], to the config

What did you expect to see?

No type error

What did you see instead?

Type '{ enabled: boolean; path: string; ignoreIncomingPaths: RegExp[]; }' is not assignable to type 'PluginConfig'.
  Object literal may only specify known properties, and 'ignoreIncomingPaths' does not exist in type 'PluginConfig'.

Additional context

If there is another way to config when using TS please add this to docs! Thanks

[naseemkullah]

Should we use ignoreUrls instead?

[dyladan]

Can you add a minimal code sample reproduction?

[naseemkullah]

Sure!

import { JaegerExporter } from '@opentelemetry/exporter-jaeger';
import opentelemetry from '@opentelemetry/core';
import { NodeTracer } from '@opentelemetry/node';
import { SimpleSpanProcessor } from '@opentelemetry/tracing';

const tracer = new NodeTracer({
  logLevel: opentelemetry.LogLevel.ERROR,
  plugins: {
    http: {
      enabled: true,
      path: '@opentelemetry/plugin-http',
      ignoreIncomingPaths: [/\/healthz/], // AND HERE
    },
  },
});

const exporter = new JaegerExporter({ serviceName: 'foo' });

tracer.addSpanProcessor(new SimpleSpanProcessor(exporter));

opentelemetry.initGlobalTracer(tracer);

[naseemkullah]

Furthermore, there is a TS error when trying to set logLevel

Type 'LogLevel.ERROR' has no properties in common with type 'PluginConfig'.ts(2559)
PluginLoader.d.ts(23, 5): The expected type comes from this index signature.

I will edit code sample above.

[dyladan]

Furthermore, there is a TS error when trying to set logLevel

Type 'LogLevel.ERROR' has no properties in common with type 'PluginConfig'.ts(2559)
PluginLoader.d.ts(23, 5): The expected type comes from this index signature.

I will edit code sample above.

To begin with, this is not valid:

const tracer = new NodeTracer({
  plugins: {
    logLevel: opentelemetry.LogLevel.ERROR, // ERROR HERE
  },
});

The log level is a configuration, not a plugin. You can do this:

const tracer = new NodeTracer({
  logLevel: opentelemetry.LogLevel.ERROR,
  plugins: {
    // http, grpc, etc
  },
});

edit: one of the examples was actually bad

For the ignoreIncomingPaths issue, there is no ignoreIncomingPaths on a plugin config object https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-types/src/trace/instrumentation/Plugin.ts#L50

The typing may need to be updated to allow plugin specific configs

[naseemkullah]

Thanks @dyladan , logLevel non-issue was me being dumb. I will edit title and code sample accordingly to just mention the ignoreIncomingPaths issue.

[dyladan]

What made you think ignoreIncomingPaths is a valid option? This is not a bug, but rather an issue with whatever documentation told you to put that there I think

[naseemkullah]

Fair enough, so it was indeed from the docs, I think I grepped for ignore one of these inspired me:
https://github.com/open-telemetry/opentelemetry-js/search?q=ignoreincomingpaths&unscoped_q=ignoreincomingpaths

FWIW it does work in regular JS... so it's a bit confusing!

I use it here https://github.com/naseemkullah/k8s-job-dispatcher/blob/master/lib/tracer.js#L12 which is my goto reference (you've since added the Getting Started guide which is amazing btw).

So we should use ignoreUrls or something else?

[dyladan]

@naseemkullah well that depends... ignoreUrls is correct according to the typing, but is actually ignored. The reason it works in js is because the config object just happens to be passed directly to the plugin unmodified, and the http plugin expects ignoreIncomingPaths. The answer is probably neither. We will need to make a better config story. For now I would use ignoreIncomingPaths with a // @ts-ignore comment to suppress compilation errors, and you should track #585 to see a real solution.

[naseemkullah]

Thanks @dyladan that is insightful as I am trying to incorporate otel into some of our TS apps.

Finally, back to the question of LogLevel, I am getting this:

    logLevel: core_1.default.LogLevel.ERROR,
                             ^

TypeError: Cannot read property 'LogLevel' of undefined

From this

import opentelemetry from '@opentelemetry/core';
import { NodeTracer } from '@opentelemetry/node';

// Create and configure NodeTracer
const tracer = new NodeTracer({
  logLevel: opentelemetry.LogLevel.ERROR,
  plugins: {
    http: {
      enabled: true,
      path: '@opentelemetry/plugin-http',
      // https://github.com/open-telemetry/opentelemetry-js/issues/585
      ignoreIncomingPaths: [/\/healthz/], // @ts-ignore
    },
  },
});

any ideas as to why?

[dyladan]

import * as opentelemetry from '@opentelemetry/core';

we have no default export. in fact i'm not sure why your script compiles at all. You probably have esModuleInterop turned on or something

[naseemkullah]

Thanks!

[OlivierAlbertini]

Thanks @dyladan , logLevel non-issue was me being dumb. I will edit title and code sample accordingly to just mention the ignoreIncomingPaths issue.

For http/https config, you can look here https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-plugin-http#http-plugin-options

Default config (interface) should be removed since it has been discussed during SIG that we don't want to have one unique config for all plugins. @mayurkale22 corrects me if I'm wrong.

[naseemkullah]

Alright // @ts-ignore works for now! Will continue to track #585