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

Add missing plugins in README.md for tracing the sample app #1509

Merged
merged 2 commits into from
Sep 21, 2020
Merged

Add missing plugins in README.md for tracing the sample app #1509

merged 2 commits into from
Sep 21, 2020

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Sep 9, 2020

When I followed this guide I got some errors after running

$ npm install \
  @opentelemetry/core \
  @opentelemetry/node \
  @opentelemetry/plugin-http

The errors:
PluginLoader#load: could not load plugin @opentelemetry/plugin-https of module https. Error: Cannot find module '@opentelemetry/plugin-https'
and
PluginLoader#load: could not load plugin @opentelemetry/plugin-express of module express. Error: Cannot find module '@opentelemetry/plugin-express'

By adding those 2 plugins to the npm install I got that step working.

I tried it with Node v10 and Node v14 and got the same results.

When I followed this guide I got some errors after running 

```sh
$ npm install \
  @opentelemetry/core \
  @opentelemetry/node \
  @opentelemetry/plugin-http
```

The errors:
PluginLoader#load: could not load plugin @opentelemetry/plugin-https of module https. Error: Cannot find module '@opentelemetry/plugin-https'
and
PluginLoader#load: could not load plugin @opentelemetry/plugin-express of module express. Error: Cannot find module '@opentelemetry/plugin-express'

By adding those 2 plugins to the `npm install` I got that step working.
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 9, 2020

CLA Check
The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Sep 9, 2020

Codecov Report

Merging #1509 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1509   +/-   ##
=======================================
  Coverage   93.82%   93.82%           
=======================================
  Files         154      154           
  Lines        4762     4762           
  Branches      951      951           
=======================================
  Hits         4468     4468           
  Misses        294      294           

@svrnm
Copy link
Member Author

svrnm commented Sep 17, 2020

@dyladan @mayurkale22 sorry for asking this stupid question, but since this is my first pull request: is there anything I need to do? Also, I found 1 or 2 other things that might need to be changed in the getting started guide: should I put everything in one big PR or open multiple?

@mayurkale22
Copy link
Member

@dyladan @mayurkale22 sorry for asking this stupid question, but since this is my first pull request: is there anything I need to do? Also, I found 1 or 2 other things that might need to be changed in the getting started guide: should I put everything in one big PR or open multiple?

I'd suggest to combine similar set of changes in one PR.

@mayurkale22 mayurkale22 merged commit 350d91c into open-telemetry:master Sep 21, 2020
mihirsoni pushed a commit to mihirsoni/opentelemetry-js that referenced this pull request Sep 21, 2020
When I followed this guide I got some errors after running 

```sh
$ npm install \
  @opentelemetry/core \
  @opentelemetry/node \
  @opentelemetry/plugin-http
```

The errors:
PluginLoader#load: could not load plugin @opentelemetry/plugin-https of module https. Error: Cannot find module '@opentelemetry/plugin-https'
and
PluginLoader#load: could not load plugin @opentelemetry/plugin-express of module express. Error: Cannot find module '@opentelemetry/plugin-express'

By adding those 2 plugins to the `npm install` I got that step working.

Co-authored-by: Mayur Kale <mayurkale@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants