-
Notifications
You must be signed in to change notification settings - Fork 0
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
findAbsolutePath
throw error with minimal React library
#247
Comments
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. |
Thanks for the feedback @jdharvey-ibm no worry for the reactivity, I understand we all face charged agendas. |
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.json Since 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 The code in question that's throwing the error is here: And it is being called by: 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:
when the files you want to analyze are in a folder called: 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 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 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. |
Thank you so much for your time and detailed explanations. |
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 Collected metrics
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 |
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 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 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. |
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. |
I believe you can close this issue. Thank you again for all the insights. 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?" |
@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 The package.json {
"dependencies": {
"my-cool-library": "^1.0.0",
"react": "^18.0.0",
"vite": "^5.3.2"
}
} The Config-wise to accomplish this, all that you should need to add in addition to the jsx scope is: # ...
collect:
+ npm:
+ dependencies:
# ... |
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. 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 ? |
Hello again.
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. 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
|
I can try to provide the changes needed to make the two examples work if that sounds good for you 🙂 |
@eric-helier this issue should be fixed in v1.6.0, let us know otherwise and thanks again for reporting 🙏🏻 |
@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.
You're definitely right that currently we only keep the anonymized version of the actual 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 :) |
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. |
findAbsolutePath
throw error with minimal React library
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 postinstall
This is part of the log with error :
Any hint on what could be the issue ?
The text was updated successfully, but these errors were encountered: