-
Notifications
You must be signed in to change notification settings - Fork 7
Opentelemetry Integration #80
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
Opentelemetry Integration #80
Conversation
|
Just talked in the weekly call:
|
|
Ok I have made some changes. You can now see the input if a specific service has caused an error. Regarding latency, the tracing service provides basically zero latency because it runs completely async with the actual services being used. Also, when application completely crashes, the spans are lost, but are restarted accordingly to bring it back up when docker engine is running. Anything else like network issues, invalid data, etc, spans are saved and tracing feature logging as no impact. |
|
I think this is a good direction. Some small changes are needed:
Also, any work in node-wot to have deeper span elements in the code? |
|
I am running npm run format, but github actions isn't saying I did... The commits that say "fix: ran npm format" is me running the command, but it isn't changing. Any way you know to fix this? Same for eslint and test. |
Please try to merge in main. Not sure if you created the PR before the prettier toolchain was fixed ... |
…ing implementation
|
Somehow package.json and package-lock.json seem to be out-of-sync |
|
@sailalithkanumuri8 merge main into this :) |
|
I have already merged main into this branch. When doing it again right now it says no changes were made. I think there is something wrong with my laptop or something because this keeps happening. Can you try to run npm install? I think it should work for you as when you did it for the docker-debugging PR the package files were up to date and checks passed. |
I think you try to merge in your local |
…ai-opentelemetry-integration
|
Checked out the PR and ran Edit: The linting errors are clearly visible now |
|
Ok I have implemented the changes requested in the call where I reduce redundancy in main.ts in advanced coffee machine and update the readme. Please let me know if there is anything else I can do to get this PR merged! |
util/util.ts
Outdated
| const getTDJSONSchema = new Promise<Record<string, unknown>>((resolve, reject) => { | ||
| https | ||
| .get( | ||
| "https://raw.githubusercontent.com/w3c/wot-thing-description/main/validation/td-json-schema-validation.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should be able to get the schema from https://github.com/w3c/wot-scripting-api/tree/main/typescript/thing-description ... see https://www.npmjs.com/package/wot-thing-description-types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok I see. I changed the code so that it uses the wot-thing-description-types now. Please let me know if there is anything else I should change.
README.md
Outdated
| const tracedThing = createAutoTracedThing(thing); | ||
|
|
||
| // Enhanced tracing with injected logic | ||
| tracedThing.setPropertyReadHandler("prop", "operation", async (logic: TracedBusinessLogic, options) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"operation" shouldn't be needed anymore
util/auto-tracing.ts
Outdated
| | ((logic: TracedBusinessLogic, options?: WoT.InteractionOptions) => Promise<T> | T), | ||
| configBuilder?: (builder: TracingConfigBuilder) => TracingConfigBuilder | ||
| ): void { | ||
| let config = tracingConfig(propertyName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also include the operation like read write etc. no?
For now, I have done the opentelemetry integration for the advanced coffee machine thing. Please let me know if I am in the right path before I proceed to integrate opentelemetry to all things.
There are a lot of file changes in the PR right now because I have merged the docker-debugging branch to this branch. Let me know if I should remove those changed files instead, but having that will be easier to test.
I use a service called Jeager to collect all the opentelemetry logs. When an action on a thing is performed, then opentelemetry will inform Jeager to make a new service and add the logs to it.
In the main.ts file in the advanced coffee thing, the code is just restructured, I didn't change much of the code. I had to do this so that I can do the tracing with opentelemetry for the specific actions.
Please let me know if there is anything I need to change or expand on. Thanks!