open-telemetry / opentelemetry-js-contrib

OpenTelemetry instrumentation for JavaScript modules
https://opentelemetry.io
Apache License 2.0
682 stars 497 forks source link

Move package-specific examples under the respective packages #552

Closed rauno56 closed 5 months ago

rauno56 commented 3 years ago

I'd like to propose moving package-specific examples(eg. examples/express) to under the package itself(eg. pkg/express), closer to the implementation, test, etc.

:+1: Less total packages to install; :+1: Shorter bootstrap time for the whole repo; :+1: More clear hierarchy to the repo; :+1: Less work and boilerplate(package.json, docs) adding an example to an instrumentation; :+1: Examples could reference unpublished ../(built source), which means that they work right after the merge and do not need a separate publish and they wouldn't be tied to a specific version - Running examples would be as easy as running tests(that's also a :-1: if you are not used to doing that);

:-1: No one place for a user who just wants to explore the examples: Could add a README pointing to some of the examples under packages or the fact that most of the packages come with examples. :-1: If ../ is referenced the example is not exactly copy-paste, because ../ has to be replaced by the package name. :-1: Running most examples requires compilation. :-1: Not all examples could reasonably be moved - some that are using multiple packages should still be left under the root examples.

dyladan commented 3 years ago

Sounds like a good idea to me. I think our examples in general need some work and I would really like it if they could be compiled in order to be sure they match the current API

obecny commented 3 years ago

I would keep examples for node the same way I did for web, one package.json and adding new example is easy, having example in each package might be not desired if we think about size etc., but it could be an improvement if you dont have to install this separately. Besides quite often for certain example you need more than just an instrumentation so it would mean keeping extra packages in every package, imho bad and hard to maintain. The easiest would be to have one node folder with one package.json and then each example will simply leave in separate folder - check this -> https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples/web and as last if we have just 2 packages to be installed - web & node then we can add this to lerna so it will be bootstrapped automatically too, so debugging etc. will be straightforward

rauno56 commented 3 years ago

Let me see if I got this right, @obecny: your suggestion would be to take all examples/*/package.json files and merge them into one examples/package.json. That would:

:+1: dedupe many of the common 3rd party libs across the examples - less total deps. :+1: keep the one place for all NodeJS examples - especially makes sense for cross-package examples. :-1: have one massive package.json with basically all OTel libs installed(eventually) + all instrumentations + all instrumented libs + bits and bobs, utilities. :-1: locks specific dep versions across the examples - prone to conflicts.

It trades off modularity, optimizing a usecase for running all examples. I think it's on par with the current setup, but the fact that over time this would get more and more difficult to manage worries me.

Flarna commented 3 years ago

As far as I know we are in general open to accept any instrumentation here as long as there is someone to maintain it. On the long run we may have >100 of them and putting samples for all of them together doesn't sound that nice for maintainaince.

A main difference from contrib vs core is that modules here are mostly independendent of each other. Additionally they often require a special setup/knowhow (e.g. database, AWS lambda,...) which is hard to combine.

I would favor to put the examples next to the instrumentations. Maybe add a generic one on top which shows a more advanced setup using several instrumentations like http + express + a database,...

svrnm commented 3 years ago

+1 for having the examples close to the instrumentation they belong to.

+1 for having some more "complex" ones at a separate place. BTW, there is already a "collection" (of one) here: https://opentelemetry.io/docs/js/instrumentation_examples/ and @niko-achilles created and shared one on slack recently: https://github.com/niko-achilles/otlp-logzio-fullstack, so this list could be extended.

As a side not on that: Ideally the README.md (or any other documentation) is kept in sync with those examples: while reviewing the website docs I found multiple instances of the same example:

https://github.com/open-telemetry/opentelemetry-js/blame/main/website_docs/getting_started/browser.md#L69 https://github.com/open-telemetry/opentelemetry-js/blame/main/packages/opentelemetry-web/README.md#L36 https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/examples/web/examples/document-load/index.js

All of them are slightly different or have been updated at a different point in time. It would be great to have only one. I am not sure if this can be solved in 100% of the cases, but maybe there are some ideas.

obecny commented 3 years ago

For node js having a single example with just one instrumentation usually doesn't make sense, some instrumentation will not even work. Plus you always need some exporter, tracing, probably docker with collector etc. etc. I would keep just simple example how to use certain instrumentation in documentation, but I would rather have a working example with more instrumentations, otherwise we will have exactly the same what we have now it will be just in different location. Ideally we would have a standalone page in with some popular todo app with backend using postgresql, graphql, memcache, redis etc. and then zipkin with collector in docker. This way it will be much easier for the end user to see what is going on with a real life example that really works and collects traces. It can be much more useful then small examples scattered now in each individual instrumentation separately.

svrnm commented 3 years ago

@obecny as just discussed on the meeting, I get your point on the "surroundings" (exporter, tracing, docker, collector, ...) that's needed for each example, but I am still not sure if I understand how that "big example" would look like, could you outline that a little bit (would there be a use case for each package that exists in https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node?)

niko-achilles commented 3 years ago

Hi Opentelementry Javascript Contributors . Thanks @svrnm for mentioning the example that i use to try in order to learn and do things with opentelemetry.

Following the discussion here and sorry for jumping in before the answer of @obecny to the question above , i am sharing with you some experience(s) that i had with repositories of opentelemetry for javascript .

a. i had to navigate to the repositories of opentelemetry-js , opentelemetry-js-contrib, opentelemetry-collector opentelemetry-collector-contrib , opentelemetry-js-api, opentelemtry-lambda in order to find things that i was interested in.

b. i had the experience that after the opentelemetry-collector realease e.g. 0.29.0 or after the usage of @opentelemetry/resources and "@opentelemetry/semantic-conventions" some examples where not updated or some examples i saw tested with otel/opentelemetry-collector image version 0.25.0 .

c. in some examples the opentelemetry packages had a version of 0.23.0 and on some examples the version 0.22.0 or 0.16.0

d. Additionally in order to find the explicit endpoint for traces and metrics i had to read the specs and then go the test implementation in order to be sure which port and which url (v1/traces) is used for HTTP otlp , grpc etc.

e. i did not use the getting started examples, and also not interested in beginner, intermediate , etc ... I was rather interested to see starter-node or starter-web with some good patterns to use and learn.

With this experience in mind , I would like to share to you some thoughts:

The leafs is good to be explicit, meaning the main force of the example. The example can use supporting forces. Antagonistic forces is a signal for a new example.

Example:

opentelemetry-javascript-examples

with-express-instrumentation (this can use as supporting libs e.g: resources, conventions , opentelemetry/api , ...) with-http-instrumenation (supporting libs: opentelemetry/api , ..) with-pg-instrumentation (supporting libs: opentelemetry/api , ..) using-collector-exporter (supporting libs: opentelemetry/api , ..) using-zipkin using-jaeger stater-web (a good example to start with .. some good dev. usage patterns ) starter-node (a good example to start with .. some good dev. patterns like node -r tracingModule app) ....

Starter node, Starter web can act as replacements to getting started, levels of difficulty beginner, etc ...

For the Fullstack example , e.g. Todo App what is the main force ? Is it e.g. distributed tracing ? Then i would expect an example

opentelemetry-javascript-examples

distributed-tracing (uses collector exporter, an example of collector config, opentelemetry/web, opentememetry/node, etc ... )

Then i can also imagine an example: opentelemetry-javascript-examples

distributed-tracing-react-node (core force react, node, the databases are supporting forces , so not in the title of the example ... )

Some final thoughts: I know that the repositories reflect on how contributors are organized and communicate with each other, so a central examples repository for javascript cannot apply to opentelemetry for javascript.

But if the people who develop e.g instrumentations in the openetelemetry-js-contrib repo can follow a minimal guide (main forces, supporting forces, antagonistic forces) on how to write an example in the central opentelemetry-javascript-examples repo, that can be possible.

With that opentelemetry for javascript will end up with a cohesive repo for examples . It is about examples after all, loose coupling because the examples are invariant (different of each other respecting the main, supporting and general forces ... )

And finally the examples can use the compatibility matrix https://github.com/open-telemetry/opentelemetry-js#about-this-project in the read me of the release of examples.

Also i would like to say that in any decision of yours about the examples i can contribute if you need support and update some of the examples now or in the future work .. i am available per email or here on github

obecny commented 3 years ago

@obecny as just discussed on the meeting, I get your point on the "surroundings" (exporter, tracing, docker, collector, ...) that's needed for each example, but I am still not sure if I understand how that "big example" would look like, could you outline that a little bit (would there be a use case for each package that exists in https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node?)

can be but it is not needed tbh, many instrumentations can be combined. How to initialise each instrumentation can be shown in readme as simple doc showing only this one particular instrumentation. At the same time you might want to have 2 different examples for one instrumentation, how would you then make it ?, For me the examples are the best entry point for user where you could copy paste things easily or just run it and see the results in zipkin for example automatically with minimum effort, npm i & npm start. For me such examples are the best to learn from, but this is just me 2 cents :)

svrnm commented 3 years ago

From what I read, I think we all agree that there should be examples that are easy to find, easy to use and good to learn from.

@obecny I think I start to understand what you consider doing, let me paraphrase in my own words:

(1) all basic examples live close by the instrumentation in the README.md, ready for a copy&paste to make them work in your own environment.

(2) one example like for web, that has many (but not all) instrumentations to play around with. I run npm i to get all packages needed and then npm start will spin up the docker containers and stuff for the surroundings?

Something like that? :-)

obecny commented 3 years ago

From what I read, I think we all agree that there should be examples that are easy to find, easy to use and good to learn from.

@obecny I think I start to understand what you consider doing, let me paraphrase in my own words:

(1) all basic examples live close by the instrumentation in the README.md, ready for a copy&paste to make them work in your own environment.

(2) one example like for web, that has many (but not all) instrumentations to play around with. I run npm i to get all packages needed and then npm start will spin up the docker containers and stuff for the surroundings?

Something like that? :-)

exactly :), just maybe running docker can be a separate command npm docker:start - what we have currently in some places

obecny commented 3 years ago

and ideally we would have an app in example for web that will report to our example in node, so that you can run frontend and backend and simply play a bit with some todo app or similar and see all traces from clicking to server with db + redis etc. etc. a working app that is fully instrumented - best real life example and also nice tool for testing anything and learning from it how to do things

svrnm commented 3 years ago

Sounds good to me, some additional thoughts:

For the basic examples it would be great to have them validated through some scripts. If they live in the README.md we could use something like the following: https://github.com/svrnm/opentelemetry-js/blob/website-review/scripts/validate-docs/index.js -- extract them from the markdown file, validate their syntax and (if possible) execute them.

For the advanced examples, the question I have is: should we have a "pure" javascript example or would it be better to have one to rule them all, e.g. something like https://codebase.show/projects/realworld ?

rauno56 commented 3 years ago

So all the library-specific(mongo, redis, restify, ..) examples from https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples would move under plugins/node/{library}/examples? That was my main problem/suggestion creating this issue.

I don't have a strong stance on where should the more complex examples live, but I don't see a good reason to move them from the current(examples/) location.

dyladan commented 3 years ago

I don't really see a reason to have a specific example for each instrumentation library. All that does is inflate the number of examples needed to be maintained.

I would prefer to see a single example which shows how to enable all instrumentations, how to enable a specific instrumentation, and how to configure an instrumentation in both cases. We would probably need one of these for node and one for web. These examples would live in the core repository, not this one.

The value of having an example for each instrumentation is primarily that we have a test app to run during development, but these test apps shouldn't need to have nice READMEs or be considered documentation. I think the test app should live in the package directory for the package being developed.

So I would see these:

If the starter examples show how to configure a plugin, then the readme from each plugin should be sufficient for a user to learn how to configure that plugin.

rauno56 commented 3 years ago

Totally agree with the last post, @dyladan. I don't see a good reason to use test-app as the folder name though instead of example - I don't think the concepts are that different.

svrnm commented 2 years ago

@open-telemetry/javascript-maintainers if this is still something we want to do, I can pick it up! I would do the following:

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] commented 2 years ago

This issue was closed because it has been stale for 14 days with no activity.

JamieDanielson commented 5 months ago

I realize this is an older issue but wanted to comment for posterity. Unfortunately we recently realized that updating dependencies was causing problems, and tried a few different ways of resolving the issues. Ultimately we decided we had to move the examples back into the top-level examples directory. I think this issue can be closed, but please comment or re-open if further discussion is needed. Please see the linked issue and PR for further detail.