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

chore: upgrade otel deps #391

Merged
merged 15 commits into from
Nov 28, 2023
Merged

chore: upgrade otel deps #391

merged 15 commits into from
Nov 28, 2023

Conversation

codecapitano
Copy link
Collaborator

@codecapitano codecapitano commented Nov 23, 2023

Why

Some users of the Web-Tracing package reported problems due to incompatible OTEL versions.
To give the package more stability we declare the necessary versions as peerDependencies.

I also manually tested it with the Faro demo and could not find any problems.

What

  • Remove outdated resolution objects and merge packages back
  • Upgrade OTEL dependencies
  • Declare OTEL dependencies as peerDependencies

Note (solved):
Some otel packages depend on a broken otel.js version which sends time as an object instead of a timestamp.
Even with forcing the newest package version, which was mentioned that it fixed it, the problem still persists.

See issues:

Status 400 because of malformed time properties:
image

Example for package which references the broken version:

"@opentelemetry/instrumentation-document-load@^0.34.0":
  version "0.34.0"
  resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-document-load/-/instrumentation-document-load-0.34.0.tgz#873075b3484fc8874f8aeb7ffcc8aac487f10dc0"
  integrity sha512-KvhQ7UDhHu/MCNeqm8rgeB82aM8D2sLYHPX65mamY9iJiXTe6cWzobZOgej5UjZqik8AE+z6mdiNupBiD7Y8lg==
  dependencies:
    **"@opentelemetry/core" "^1.8.0"**
    "@opentelemetry/instrumentation" "^0.45.1"
    "@opentelemetry/sdk-trace-base" "^1.0.0"
    "@opentelemetry/sdk-trace-web" "^1.15.0"
    "@opentelemetry/semantic-conventions" "^1.0.0"

Update:
The issue is fixed.
Screenshot 2023-11-27 at 11 54 53

We can either wait till the packages use the newest version or downgrade.
Thoughts?

cc @cedricziel

Links

Checklist

  • Tests added
  • Changelog updated
  • Documentation updated

@codecapitano codecapitano marked this pull request as draft November 24, 2023 10:36
yarn.lock Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@codecapitano
Copy link
Collaborator Author

Hey @SimenB just a quick update:

The problem with sending timings in LongBits instead of a string/number is solved.
It was not enough to enable useHex but also to explicitly disable useLongBits in createExportTraceServiceRequest.

https://vscode.dev/github/grafana/faro-web-sdk/blob/upgrade-otel-deps/packages/web-tracing/src/faroTraceExporter.ts#L12

@codecapitano codecapitano marked this pull request as ready for review November 27, 2023 10:56
Copy link
Contributor

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

😍

packages/web-tracing/package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw 😀

@codecapitano codecapitano added bug Report a bug dependencies Pull requests that update a dependency file and removed improvement Request a change of an existing feature labels Nov 27, 2023
@codecapitano codecapitano merged commit beb7faf into main Nov 28, 2023
2 checks passed
@codecapitano codecapitano deleted the upgrade-otel-deps branch November 28, 2023 08:46
@SimenB
Copy link
Contributor

SimenB commented Nov 28, 2023

👏

Would love to see a publish as well 🙏

@codecapitano
Copy link
Collaborator Author

👏

Would love to see a publish as well 🙏

Hey Simen, we plan to release a new version tomorrow (Wednesday).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report a bug dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Declare specified OTel package versions as a peer dependency to prevent problems caused by different versions
2 participants