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 2022/11/14 15:14:29 UTC
[GitHub] [skywalking-nodejs] tom-pytel opened a new pull request, #102: Update AxiosPlugin for v1.0+
tom-pytel opened a new pull request, #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102
Old plugin doesn't work with v1.0+, dropping support for < v1.0. Tests will probably break so will see how to fix.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] kezhenxu94 commented on a diff in pull request #102: Update AxiosPlugin for v1.0+
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102#discussion_r1022261607
##########
src/plugins/AxiosPlugin.ts:
##########
@@ -26,20 +26,39 @@ import { SpanLayer } from '../proto/language-agent/Tracing_pb';
import DummySpan from '../trace/span/DummySpan';
import { ignoreHttpMethodCheck } from '../config/AgentConfig';
import PluginInstaller from '../core/PluginInstaller';
+import * as fs from 'fs';
+import * as path from 'path';
class AxiosPlugin implements SwPlugin {
readonly module = 'axios';
readonly versions = '*';
+ getVersion(installer: PluginInstaller): string {
+ // TODO: this method will not work in a bundle
+ try {
+ const indexPath = installer.resolve(this.module);
+ const dirname = indexPath.slice(
+ 0,
+ indexPath.lastIndexOf(`${path.sep}node_modules${path.sep}axios${path.sep}`) + 20,
+ );
+ const packageSJonStr = fs.readFileSync(`${dirname}package.json`, { encoding: 'utf-8' });
+ const pkg = JSON.parse(packageSJonStr);
Review Comment:
```suggestion
const packageJsonStr = fs.readFileSync(`${dirname}package.json`, { encoding: 'utf-8' });
const pkg = JSON.parse(packageJsonStr);
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on a diff in pull request #102: Update AxiosPlugin for v1.0+
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on code in PR #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102#discussion_r1022647753
##########
src/plugins/AxiosPlugin.ts:
##########
@@ -26,20 +26,39 @@ import { SpanLayer } from '../proto/language-agent/Tracing_pb';
import DummySpan from '../trace/span/DummySpan';
import { ignoreHttpMethodCheck } from '../config/AgentConfig';
import PluginInstaller from '../core/PluginInstaller';
+import * as fs from 'fs';
+import * as path from 'path';
class AxiosPlugin implements SwPlugin {
readonly module = 'axios';
readonly versions = '*';
+ getVersion(installer: PluginInstaller): string {
+ // TODO: this method will not work in a bundle
+ try {
+ const indexPath = installer.resolve(this.module);
+ const dirname = indexPath.slice(
+ 0,
+ indexPath.lastIndexOf(`${path.sep}node_modules${path.sep}axios${path.sep}`) + 20,
+ );
+ const packageSJonStr = fs.readFileSync(`${dirname}package.json`, { encoding: 'utf-8' });
+ const pkg = JSON.parse(packageSJonStr);
Review Comment:
🥇 :|
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #102: Update AxiosPlugin for v1.0+
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on PR #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102#issuecomment-1313925064
Newest Axios seems to be incompatible with Node 10. Suggest dropping Node 10 support since lowest Node currently getting security updates is 14 and latest version is 19.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel commented on pull request #102: Update AxiosPlugin for v1.0+
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on PR #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102#issuecomment-1313975061
> Are we too aggressive to drop the old version release? @kezhenxu94 Do we have a way to build plugin for various versions? As a result, we could keep the plugin for 0.x and one more for 1.x
Not a big problem to leave it compatible with old Axios for old Node 10, this should work for all versions, lets see what tests say. I left test version as old Axios because otherwise Node 10 test would fall over and die.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] tom-pytel merged pull request #102: Update AxiosPlugin for v1.0+
Posted by GitBox <gi...@apache.org>.
tom-pytel merged PR #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-nodejs] wu-sheng commented on pull request #102: Update AxiosPlugin for v1.0+
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #102:
URL: https://github.com/apache/skywalking-nodejs/pull/102#issuecomment-1313930424
v1.0 is released one month ago, https://github.com/axios/axios/releases/tag/v1.0.0
Are we too aggressive to drop the old version release? @kezhenxu94 Do we have a way to build plugin for various versions? As a result, we could keep the plugin for 0.x and one more for 1.x
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org