You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/05/14 16:32:44 UTC
[GitHub] [skywalking-nodejs] tom-pytel opened a new pull request #51: add AzureHttpTriggerPlugin
tom-pytel opened a new pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51
This is a work-in-progress that could use some input. As discussed with @wu-sheng this is the Azure Functions HttpTrigger instrumentation plugin. It allows traces to include these endpoints both as final as well as passing through to other endpoints. It doesn't make too much sense to profile them, but having them included rather than invisible as part of a trace might be useful.
The work-in-progress part is not the plugin itself, it works, but rather how it integrates with SW. For example currently I have it set as component `Component.HTTP_SERVER` rather than its own Azure component name, should that change? And should it have its own Azure logo? Which may be shared in the future with other Azure Function instrumented trigger functions like RabbitMQ? Or should they differentiate? Also this requires manual instrumentation (although very easy), how do you guys feel about that?
There is also the question of a CI test which may be a bit complicated to do and I can't really look into that at at the moment since I have a few other things which require my attention. Would need a docker image with Azure CLI tools to run local.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] kezhenxu94 commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841621600
> I've added a new version code for a plugin of "!" to indicate that the plugin does not check version on install since it is applied manually.
I’m wondering how the codes look like if users want to manually install the plugin. Can we have a dedicated directory (that’s not installed automatically) to put this kind of plugins?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841657739
> > Ha, half instrumentation could happen. But half auto-instrumentation is auto-instrumentation. You actually just don't want to install in any case, right?
>
> You mean since half-instrumentation needs normal plugin init to happen that it would live in the plugin directory as opposed to this "plugin" which does not have anything to automatically init, it is completely manual?
Yes, a new folder @kezhenxu94 proposed here, I think it should be only for 100% manual. Make sense? Half auto-instrumentation or real auto-instrumentation should leave to the documentation and coding details
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] kezhenxu94 merged pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841655957
> Ha, half instrumentation could happen. But half auto-instrumentation is auto-instrumentation. You actually just don't want to install in any case, right?
You mean since half-instrumentation needs normal plugin init to happen that it would live in the plugin directory as opposed to this "plugin" which does not have anything to automatically init, it is completely manual?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841603595
> currently I have it set as component Component.HTTP_SERVER rather than its own Azure component name, should that change?
I think once this is for AZure only, we should have a new component for it.
> And should it have its own Azure logo? Which may be shared in the future with other Azure Function instrumented trigger functions like RabbitMQ? Or should they differentiate?
I think we should have an Azure logo, to indicate this is user's FaaS logic service/function. RabbitMQ could be shared, even that is Azure hosted MQ service.
> Also this requires manual instrumentation (although very easy), how do you guys feel about that?
If there is no chance to be auto-instrumentation, it is fine as manual. We have manual agent/SDK plugins in other language agents too.
> I've added a new version code for a plugin of "!" to indicate that the plugin does not check version on install since it is applied manually.
@kezhenxu94 WDYT?
> There is also the question of a CI test which may be a bit complicated to do and I can't really look into that at at the moment since I have a few other things which require my attention. Would need a docker image with Azure CLI tools to run local.
CI is recommended, if possible. We could delay it a little if your time is limited for now. But once someone uses this and faces bug, you have to recheck locally. User can't tell the difference between your use case and their own.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel edited a comment on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
tom-pytel edited a comment on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841687997
Removed new version code '!' and moved AzureHttpTriggerPlugin to new dir `azure/`. This may be temporary or could be changed in the future to a different dir which contains all manually installed plugins or it may stay specific to azure, not important since the plugin is imported at the top level so path is hidden from user.
Also added new component ID for AzureHttpTrigger which may be shared among the various languages Azure Functions support and added "Azure Functions" logo icon in their respective projects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] kezhenxu94 merged pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841646458
> I’m wondering how the codes look like if users want to manually install the plugin. Can we have a dedicated directory (that’s not installed automatically) to put this kind of plugins?
That is a better idea, will do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841654877
> > > I’m wondering how the codes look like if users want to manually install the plugin. Can we have a dedicated directory (that’s not installed automatically) to put this kind of plugins?
> >
> >
> > That is a better idea, will do.
>
> Actually, just had a thought, what if in the future there is a plugin that is part automatic and part manual, that needs plugin init but also requires user instrumentation of some part?
Ha, half instrumentation could happen. But half auto-instrumentation is auto-instrumentation. You actually just don't want to install in any case, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841651138
> > I’m wondering how the codes look like if users want to manually install the plugin. Can we have a dedicated directory (that’s not installed automatically) to put this kind of plugins?
>
> That is a better idea, will do.
Actually, just had a thought, what if in the future there is a plugin that is part automatic and part manual, that needs plugin init but also requires user instrumentation of some part?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #51: add AzureHttpTriggerPlugin
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on pull request #51:
URL: https://github.com/apache/skywalking-nodejs/pull/51#issuecomment-841687997
Removed new version code '!' and moved AzureHttpTriggerPlugin to new dir `azure/`. This may be temporary and could be changed in the future to a different dir which contains all manually installed plugins or it may stay specific to azure, not important since the plugin is imported at the top level so path is hidden from user.
Also added new component ID for AzureHttpTrigger which may be shared among the various languages Azure Functions support and added "Azure Functions" logo icon in their respective projects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org