-
Notifications
You must be signed in to change notification settings - Fork 538
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
Move package-specific examples under the respective packages #552
Comments
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 |
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 |
Let me see if I got this right, @obecny: your suggestion would be to take all 👍 dedupe many of the common 3rd party libs across the examples - less total deps. 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. |
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. |
+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: https://github.com/open-telemetry/opentelemetry-js/blame/main/website_docs/getting_started/browser.md#L69 All of them are slightly different or have been updated at a different point in time. It would be great to have only one. |
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. |
@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?) |
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 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 ( 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 Example: opentelemetry-javascript-examples
For the opentelemetry-javascript-examples
Then i can also imagine an example:
Some final thoughts: 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 |
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. |
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 Something like that? :-) |
exactly :), just maybe running docker can be a separate command |
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 |
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 ? |
So all the library-specific(mongo, redis, restify, ..) examples from https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples would move under 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( |
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. |
Totally agree with the last post, @dyladan. |
@open-telemetry/javascript-maintainers if this is still something we want to do, I can pick it up! I would do the following:
|
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. |
This issue was closed because it has been stale for 14 days with no activity. |
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. |
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.
👍 Less total packages to install;
👍 Shorter bootstrap time for the whole repo;
👍 More clear hierarchy to the repo;
👍 Less work and boilerplate(package.json, docs) adding an example to an instrumentation;
👍 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 👎 if you are not used to doing that);👎 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.
👎 If
../
is referenced the example is not exactly copy-paste, because../
has to be replaced by the package name.👎 Running most examples requires compilation.
👎 Not all examples could reasonably be moved - some that are using multiple packages should still be left under the root examples.
The text was updated successfully, but these errors were encountered: