Integrating openTelemetry in old applications

[OlivierAlbertini]

As discussed in SIG meeting

I started to integrate OpenTelemetry in our applications. It works great for new apps but I faced some issues in our old applications.

Indeed, we have a lot of micro services that use Typescript version < 3.0.0. The behaviour is that tsc try to compile *.d.ts file inside the node_modules even with some tsconfig options like

"skipLibCheck": true,
 [...]
"exclude": [
    "node_modules",
    "dist"
 ]

I was able to solve this issue by upgrading typescript/tslint
But It costs a bit more to integrate OpenTelemetry for old applications since upgrading typescript generate other issues as we have experimented in this repo when we have upgraded to 3.7.2

In order to reproduce the issue, I created a branch in our forked repo. I converted the http example by using Typescript version 2.8.4.

https://github.com/VilledeMontreal/opentelemetry-js/tree/feature/typecript-issue

In examples/http folder, you will need to do:

• npm i
• ./node_modules/.bin/tsc -p ./tsconfig.json

You should see:

../../packages/opentelemetry-core/build/src/platform/node/performance.d.ts:17:43 - error TS1005: ',' expected.

17 export declare const otperformance: import("perf_hooks").Performance;
                                             ~

[dyladan]

That feature was introduced in typescript 2.9 https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-9.html#import-types

Using import("mod") in a type annotation allows for reaching in a module and accessing its exported declaration without importing it.

Explanation of the issue

Basically if you look at the following code:

import { performance } from 'perf_hooks';
export const otperformance = performance;

When it is compiled, it emits the following declaration (d.ts) file:

/// <reference types="node" />
export declare const otperformance: import("perf_hooks").Performance;

Basically, it is importing the type without actually importing the file. If you try to compile this with a version of typescript previous to 2.9 it fails to compile completely, complaining about Exported variable 'X' has or is using name 'Y' from external module. Basically what it means, is that the exported otperformance variable is of a type that isn't actually imported in the file. You can fix it by adding Performance import to the top of the file and adding the type explicitly to the otperformance variable.

import { performance, Performance } from "perf_hooks";
export const otperformance: Performance = performance;

Which compiles successfully with ts versions before 2.9 and emits the following declaration (with 3.7):

/// <reference types="node" />
import { Performance } from "perf_hooks";
export declare const otperformance: Performance;

This declaration is understood by older versions of TS and everyone is happy.

What does this mean?

We can avoid this behavior by being very careful to only write type files that explicitly import any types they use in an export type, like the example above. A cursory look at the dist/ folders in my local version shows about 10 offending declarations which could all be modified to explicitly export their types.

But this is really just one small part of the issue which is that we are using the latest version of typescript to compile the library, and our users may be using versions which do not understand the output declarations.

As I see it these are our options:

1. downgrade our version of typescript
2. do our best to write typescript that emits in backwards compatible ways
3. document that users may need to upgrade their typescript versions

Prominent example DefinitelyTyped does a version of 1. They test all packages on all versions of typescript 2.8 and later, and tag releases in npm with tags like ts2.9 and ts3.2 showing which versions it is known to be compatible with.

(2) is basically impossible as we can't expect every developer to be an expert in compiler output and which typescript versions will be able to read their declarations

[OlivierAlbertini]

I would not recommend option 1.

For option 2, we could require automated tests in order to be sure that we are compatible with Typescript versions that we want to support. We could add in the CONTRIBUTING.md that we need to develop packages that are compatibles with X versions of TypeScript. If we are aware of this requirement we can take care of this during development/review process. WDYT ?

For option 3, it will be good for all Typescript versions that we don't support.

Also I try to check if there is a way to ignore d.ts files for specific packages. I can't believe that writing Typescript packages can effect Typescript projects that use these packages because of *.d.ts files... I hope that there is a way to tell to the compiler that if you don't understand the *.d.ts files you won't have the Typescript features for these packages but you can work on your app and deploy without issue...
Perhaps with the "types" in the "compilerOptions" (there is something todo as workaround)

[dyladan]

The problem with (2) is that we lose the ability to use some newer features of typescript. Maybe we can make a very simple sample project that just loads all of the packages and compiles, and run it against each version of typescript in the circleci? It would be very fast since it would not need to install any deps other than typescript and @opentelemetry/*

edit: we can even use the http example for this

[OlivierAlbertini]

The problem with (2) is that we lose the ability to use some newer features of typescript. Maybe we can make a very simple sample project that just loads all of the packages and compiles, and run it against each version of typescript in the circleci? It would be very fast since it would not need to install any deps other than typescript and @opentelemetry/*

edit: we can even use the http example for this

That's what I was thinking when I wrote

For option 2, we could require automated tests in order to be sure that we are compatible with Typescript versions that we want to support. We could add in the CONTRIBUTING.md that we need to develop packages that are compatibles with X versions of TypeScript. If we are aware of this requirement we can take care of this during development/review process. WDYT ?

So next step would be to create an issue (e.g Add Typescript tests during circleci build) in order to move on or should we wait and discuss about that during the next SIG meeting ?

[dyladan]

I think this is a SIG topic. My gut feeling is that this is something that should wait for beta or even 1.0 because it is likely to slow down development and requiring recent versions of ts isn't unreasonable for alpha software.

[Flarna]

Prominent example DefinitelyTyped does a version of 1. They test all packages on all versions of typescript 2.8 and later, and tag releases in npm with tags like ts2.9 and ts3.2 showing which versions it is known to be compatible with.

To my knowledge releases of DT are not tagged with typescript versions at NPM. The packages have a comment like // TypeScript Version: 2.8 which indicates to their tools which min typescript version should be used for testing (since a month or two the global min was moved from 2.0 to 2.8).

Since 3.1 typescript supports versioning definitions withing one package using the typesVersions field in package.json.
This is for example used @types/node, so it's consumable with tsc 2.8 but if you use 3.2 you get e.g. some more APIs with bigint. But it's still a single NPM package and both variants need to be maintained (by hand) in parallel.

Packages here are written in typescript, therefore type definitions are generated automatically. I'm not aware of any tooling to create versioned d.ts files using typescript.

[dyladan]

We can add an “old ts versions” recommendation to the readme explaining how to ignore the generated types

per sig

[dyladan]

Some more context from the SIG. It was decided that this isn't a priority for alpha. If we can find a way to make old ts versions ignore the types or override them with an any for old ts versions, it should be sufficient to add a recommendation in the README explaining how to do so. Users will lose the typing information, but old ts versions are not a priority.

[pauldraper]

The TS community has this problem generally, and there isn't a great solution.

In practice it's fairly minimal as upgrading TS is rather easy (some adjusting of flags, type annotations...but unlikely to cause any runtime errors).

[pichlermarc]

We've determined that we'll not support older typescript versions and we'll even move on to only supporting the latest typescript version in 2.0. #3955

This is due to the massive maintenance effort that supporting older versions incurs.