-
Notifications
You must be signed in to change notification settings - Fork 57
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
Added jaeger-agent as known sidecar #294
Conversation
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.
looks good to me. I would suggest testing this fully before merging and releasing it. Because I feel there might some other places in the code might need tweaks. Thanks @BartNetJS that was fast :) !!
Hi @hsubramanianaks, Do you expect me to do something to get it approved by your colleagues? |
@BartNetJS I can generate a VSIX with your fix and you can test it with Jeager sidecars to see if it works ? So that we can confirm it works fully. Please let me know. After this we can ask other code owners to review and give their comments/approval. |
@BartNetJS here is the VSIX - https://github.com/hsubramanianaks/vscode-bridge-to-kubernetes/releases/tag/jeager-support, please set BRIDGE_ENVIRONMENT='dev' and then install VSIX and run your apps with Jeager to see if it works. Thank you. |
The vsix gives an error: 17/07/2023 10:41:23 - Microsoft VSIX Installer If posisble can you give a vscode extension? |
This vsix is for vscode extension not Visual studio. Sorry I should have mentioned this. Please try and let me know. @BartNetJS |
@hsubramanianaks I got his popups when i try to debug(local tunnel) |
@BartNetJS you need to set BRIDGE_ENVIRONMENT='dev' before launching and using your vscode and vscode extension. It should be available as environment variables. |
@hsubramanianaks forgot to set it, sorry for that |
@cxznmhdcxz can you do internal round of testing and confirm on the results please? |
Passed on all 3 platforms |
See also #292
This PR allows Jaeger side car support by bridge.