Closed eric-helier closed 3 months ago
jharvey10
commented
3 months ago Hi @eric-helier! This issue slipped by without me noticing, so I'm super sorry about that. Now that I see it, I just wanted to quickly say thanks for your interest in the project and that I'm going to spend some time today or tomorrow to understand the issue you're seeing and see what I can do to help.
eric-helier
commented
3 months ago Thanks for the feedback @jdharvey-ibm no worry for the reactivity, I understand we all face charged agendas. If my issue is difficult to understand / reproduce I can try to give more details.
jharvey10
commented
3 months ago Thanks for your patience. I'm doing some digging on this trying to get this error to pop out in my environment as well.
One thing investigating this led me to find was that our readme was slightly out of date. The schema line in the config file can instead look like this:
# yaml-language-server: $schema=https://unpkg.com/@ibm/telemetry-config-schema@1/dist/config.schema.jsonSince we are interface stable with our config schema yaml at this point. This won't affect your issue, but it will unlock some additional scopes and options for collection. Namely: our js scope in addition to jsx and npm.
The code in question that's throwing the error is here: https://github.com/ibm-telemetry/telemetry-js/blob/main/src/main/core/tracked-file-enumerator.ts
And it is being called by: https://github.com/ibm-telemetry/telemetry-js/blob/main/src/main/core/get-tracked-source-files.ts
A couple things to note: After discovering source files in a project using Git, we construct absolute paths to those files to use throughout the rest of the tooling. After construction, the way we check if those absolute paths are correct is by using node's access function. Here, that's the check that's failing and causing the errors to be seen.
One possible reason for that could be that the telemetry node process does not have permission to those files. If that's the case, then the tooling won't be able to discover/analyze any source files.
There's another thing to note about pathing: When you install a dependency locally, you typically end up with a symlink in your node_modules folder to the actual location of the dep. That won't work for telemetry because the path of the dep in question will be outside of the project being analyzed, so you'll see errors showing the tooling looking for dependency and other data outside of the project you're trying to analyze. You might see things like:
error 18725 2024-06-19T21:08:18.038Z T_e Error: No package installation found for @receter/my-component-library
at file:///Users/jdharvey/Desktop/stuffwow/my-component-library/node_modules/@ibm/telemetry-js/dist/background-process.js:473:21527
at async Promise.all (index 2)
at async k_e (file:///Users/jdharvey/Desktop/stuffwow/my-component-library/node_modules/@ibm/telemetry-js/dist/background-process.js:473:21581)
at async u.captureAllMetrics (file:///Users/jdharvey/Desktop/stuffwow/my-component-library/node_modules/@ibm/telemetry-js/dist/background-process.js:473:40200)when the files you want to analyze are in a folder called: /Users/jdharvey/Desktop/stuffwow/vite-project.
And your library is in a folder called: /Users/jdharvey/Desktop/stuffwow/my-component-library.
Said differently, it's using the "wrong" node_modules folder for all of its comparisons.
For local testing, I believe what we ended up doing was to build our instrumented component library and then run npm pack to get a zip of it. And then we'd install the zip into the project so that the files physically exist in the node_modules folder.
One more side note: The code as it is right now relies on there being an initialized git repo in which to run as well as a remote named origin configured. We have a TODO to revisit that logic, but for now, that's a potential pitfall.
Hopefully some of this helps! If you're still stuck, feel free to provide more info about what you see and I'm happy to help.
eric-helier
commented
3 months ago Thank you so much for your time and detailed explanations. I'll have a look at it again and get back to you with my results. We are really interested in this approach as that could solve a major issue we have currently in making visible real usage metrics of our inner-source libraries.
eric-helier
commented
3 months ago I think what was breaking was missing origin on consumer project. I pushed the project to a remote git repository, and now it looks to work. The output show that the "Button" component published from instrumented library has been found in consumer project.
Here is the output for the Button component with size and variant props. I'm super excited to get it working and thank you again for the hints.
Now I'll try to figure out how to run an opentelemetry endpoint locally.
Also a question on telemetry config, is there a way to allow wildcards or generalized values for allowedAttributeNames and allowedAttributeStringValues ?
Maybe I missed something in the documentation around that part of the configuration.
I will give a try at using https://www.npmjs.com/package/@ibm/telemetry-js-config-generator also.
jharvey10
commented
3 months ago Awesome! Glad you got some button metrics out of it!
The quickest way I was able to get an OTel collector running was by creating a free trial to the Elastic stack since they have a native OpenTelemetry integration via the APM Integration.
If you just want to fiddle around with requests, you basically just need to run an API endpoint that listens for POST requests on /v1/metrics. What you do with the incoming data at that point is totally up to you!
Regarding the config file, you didn't miss anything. One of the most critical aspects of this project is its protection of sensitive data. Since we use this tooling to collect data from other people's code repos, we take their code's privacy and confidentiality extremely seriously, even though that means making things a bit less convenient for those who might use this in a closed-source environment or something similar.
In terms of configuration, what that means is that the telemetry-js package is built in a way where there are no off switches for our critical security features, namely anonymization, de-identification, and allowlisting over blocklisting where sensitive information may be present.
We have taken the stance that the person instrumenting their package must explicitly define values that are 'okay' to exist in plain text in the output metrics so as to prevent our back-end data warehouse from ever being contaminated with PI, SPI, or confidential information. This mainly applies to string values being used in props or function parameters.
We fully realize that this is inconvenient for the maintainers, but given our small dev team, it has been the optimal decision for us in this regard. That's where the config generator comes into play. We want to assist in the generation of those value sets without "opening the flood gates."
With that said, if there's any aspect of the structure of the config schema that could be improved based on your use cases, we're definitely open to that. We've gotten feedback from internal teams that having to define basically the same list twice is kind of a pain, and also that if values could be specified per-component, it would make maintenance and automation a bit easier.
eric-helier
commented
3 months ago Totally makes sense. the strong anonymisation and de-identification approach may actually be a good thing for us, since in Airbus there are very strict rules that would block us from using telemetry otherwise.
I'm in contact with a team internally maintaining Splunk on premise. Looks like splunk supports OpenTelemetry protocol so that could be a win.
In the meantime I'll fiddle with a self made API endpoint like you suggest, just to play with the toolkit and configuration.
eric-helier
commented
3 months ago I believe you can close this issue. Thank you again for all the insights. I've setup a basic nodejs express server to demonstrate the kind of data we can get, works perfectly.
A last question maybe : for the npm dependencies, it is perfect to get the version number of the instrumented lib. But is it possible to specify a list of other dependencies for which we would like to know if it is used and with what version ? (I'm thinking of dependencies like react/react-dom/vite/jest/mui...). I don't see in the telemetry config schema any way to specify such a "white list" of npm dependencies : https://github.com/ibm-telemetry/telemetry-config-schema?tab=readme-ov-file#npm-scope
Looks possible from reading the doc : "What version of React/Angular/Vue... are consumers most using along with my package?"
jharvey10
commented
3 months ago @eric-helier Awesome! Glad you're getting some usefulness out of this project! That helps validate for me that we're heading in the right direction.
Regarding the npm scope: This one is almost the opposite of the jsx scope in terms of how much it collects by default. I'll start out by saying that I'm definitely going to make an issue to let users specify specific packages to collect as a part of the npm scope, but the purpose of that may surprise you :) It's to limit data collection, not increase it 😄. We've actually considered extending the config that way before.
The npm scope by default will look at where the instrumented package was installed, and capture one metric for every package found that is adjacent to the instrumented package. So for example, if you have a project that looks like this:
package.json
{
"dependencies": {
"my-cool-library": "^1.0.0",
"react": "^18.0.0",
"vite": "^5.3.2"
}
}The npm scope will [by default] capture 3 metrics. One for my-cool-library, one for react, and one for vite, along with their versions. my-cool-library will have one extra field (npm.dependency.isInstrumented), and it will be set to true so you can distinguish them from each other.
Config-wise to accomplish this, all that you should need to add in addition to the jsx scope is:
# ...
collect:
+ npm:
+ dependencies:
# ...I'll start out by saying that I'm definitely going to make an issue to let users specify specific packages to collect
That would be a great feature 👍
Also maybe there could be ways to reduce the output sent to opentelemetry protocol to a more lightweight data set. I've tested our real react library on a dozen real projects. npm scope output vary around 200k and jsx output around 1/2mb.
That's quite big as the huge majority of the data is "unreadable", maybe there could be an option to only send readable information ?
Another question regarding project's information, would that be ok to provide a way to get the package.json name entry ? Currently as I understand we only get an anonymised name (I wrongly thought initially that project.id would be that, but it's the instrumented library project id). How do you manage on your side to get an understanding of which project is attached to some collected data ? Or do you simply ignore that level of information ?
eric-helier
commented
3 months ago Hello again. Regarding the initial error of this issue, I think I've found a reproducible use case that throw this error. This is also easy to test with unit tests of this method https://github.com/ibm-telemetry/telemetry-js/blob/9695b5199ac8e583f12b94a37f4f1680692d5b28/src/main/core/tracked-file-enumerator.ts#L66
Here was my parameters for root and trackedFile
In this case, the finalPath value is "/Users/helier_e/Dev/projects/someproject-xxxx-cockpit/config/env.js" which is wrong. it should be "/Users/helier_e/Dev/projects/someproject-xxxx-cockpit/cockpit/config/env.js"
I've been able to workaround this issue by renaming the project folder to "someproject-xxxx" (removing the "cockpit" from the end of the folder name).
I believe this could happen quite frequently, I've currently tested the approach on a dozen real projects, and two of them had this issue.
Other example
eric-helier
commented
3 months ago I can try to provide the changes needed to make the two examples work if that sounds good for you 🙂
francinelucca
commented
3 months ago @eric-helier this issue should be fixed in v1.6.0, let us know otherwise and thanks again for reporting 🙏🏻
jharvey10
commented
2 months ago That's quite big as the huge majority of the data is "unreadable", maybe there could be an option to only send readable information ?
@eric-helier That's a great suggestion! I'm gonna add that to our project board for the next time we do a prioritization exercise. You're definitely right that it's a lot of data.
Another question regarding project's information, would that be ok to provide a way to get the package.json name entry ? Currently as I understand we only get an anonymised name (I wrongly thought initially that project.id would be that, but it's the instrumented library project id). How do you manage on your side to get an understanding of which project is attached to some collected data ? Or do you simply ignore that level of information ?
You're definitely right that currently we only keep the anonymized version of the actual name field from the package.json file and use the npm.dependency.isInstrumented field to determine which of the incoming metrics represents the instrumented package.
The purpose of the projectId is mostly to outlast renames/restructurings of packages or one package superseding another. I wanted the data to all still be relatable even if a package was renamed or restructured for some reason. We've seen that happen a couple times in our package ecosystem, and I wanted the flexibility to tolerate that at the earliest point possible. So we assign a projectId to every instrumented package, and along with that ID, we store all known names of the package in plain text, the maintainer, and some other metadata. It's just a basic NoSQL database, but that's what allows us to make that correlation.
Generally speaking, when you "collect" the data on the server side, you can add to it however makes the most sense to you. That could mean looking up an incoming hash in a set of known hashes and adding some human-readable fields prior to storage. That could also be done periodically as a batch job or something, too. There's also some internal IDs that we attach to certain repos and projects prior to storing them in our data warehouse to make future lookup and correlation more convenient.
With all of that said, I'm going to add another entry on our project board to simply consider adding a "project.name" field to the transmitted data :)
eric-helier
commented
2 months ago Thanks for the answer. FYI I already added two separates entries to the github issues, so you can close this one and decide what to do for the two other topics
As stated in the two issues, we are also motivated to collaborate on these features and maybe contribute some changes.
Description
I'm evaluating the feasibility to use telemetry-js to track usage of Airbus innersource design system react library, with a potential internal opentelemetry collector. Studying with a minimal react library build with vite, I'm getting error thrown when running ibmtelemetry scripts to collect data.
Steps to reproduce
CI=true npm explore @airbus/poc-telemetry-reactlib -- npm run postinstallThis is part of the log with error :
Any hint on what could be the issue ?