Skip to content
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

Stops encouraging serviceName overrides #389

Merged
merged 2 commits into from
Jul 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/zipkin-instrumentation-axiosjs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const localServiceName = 'service-a'; // name of this application
const tracer = new Tracer({ ctxImpl, recorder, localServiceName });

const remoteServiceName = 'weather-api';
const zipkinAxios = wrapAxios(axios, { tracer, serviceName: localServiceName, remoteServiceName });
const zipkinAxios = wrapAxios(axios, { tracer, remoteServiceName });

zipkinAxios.get('/user?ID=12345')
.then(function (response) {
Expand All @@ -46,11 +46,7 @@ zipkinAxios.get('/user?ID=12345')
timeout: 1000,
headers: {'X-Custom-Header': 'foobar'}
});
axiosInstance = wrapAxios(axiosInstance, {
tracer,
serviceName: localServiceName,
remoteServiceName
});
axiosInstance = wrapAxios(axiosInstance, {tracer, remoteServiceName});
```


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ describe('axios instrumentation - integration test', () => {

beforeEach(() => {
spans = [];
tracer = new Tracer({ctxImpl: new ExplicitContext(), recorder: newSpanRecorder(spans)});
tracer = new Tracer({
ctxImpl: new ExplicitContext(),
localServiceName: serviceName,
recorder: newSpanRecorder(spans)
});
});

function popSpan() {
Expand All @@ -52,7 +56,7 @@ describe('axios instrumentation - integration test', () => {
timeout: 300 // this avoids flakes in CI
});

return wrapAxios(instance, {tracer, serviceName, remoteServiceName});
return wrapAxios(instance, {tracer, remoteServiceName});
}

function url(path) {
Expand Down
6 changes: 3 additions & 3 deletions packages/zipkin-instrumentation-axiosjs/test/unitTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ describe('axios instrumentation - unit test', () => {
const record = sinon.spy();
const recorder = {record};
const ctxImpl = new ExplicitContext();
tracer = new Tracer({recorder, ctxImpl});
tracer = new Tracer({recorder, localServiceName: serviceName, ctxImpl});
});
it('should return an axios instance when pass axios', done => {
const axiosInstance = wrapAxios(axios, {tracer, serviceName, remoteServiceName});
const axiosInstance = wrapAxios(axios, {tracer, remoteServiceName});
expect(axiosInstance.create).to.equal(undefined);
done();
});

it('should return itself when pass an axiosInstance', done => {
const axiosInstance = axios.create();
const zipkinAxiosInstance = wrapAxios(axiosInstance, {tracer, serviceName, remoteServiceName});
const zipkinAxiosInstance = wrapAxios(axiosInstance, {tracer, remoteServiceName});
expect(axiosInstance).to.equal(zipkinAxiosInstance);
done();
});
Expand Down
36 changes: 18 additions & 18 deletions packages/zipkin-instrumentation-connect/test/integrationTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ const middleware = require('../src/middleware');
const https = require('https');
const fs = require('fs');

const serviceName = 'service-a';
const serviceName = 'weather-app';
const testSetup = () => {
const record = sinon.spy();
const recorder = {record};
const ctxImpl = new ExplicitContext();
const tracer = new Tracer({recorder, ctxImpl});
const tracer = new Tracer({recorder, localServiceName: serviceName, ctxImpl});
return {record, recorder, ctxImpl, tracer};
};

Expand All @@ -23,7 +23,7 @@ describe('restify middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = restify.createServer();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.post('/foo', (req, res, next) => {
ctxImpl.scoped(() => {
// Use setTimeout to test that the trace context is propagated into the callback
Expand Down Expand Up @@ -58,7 +58,7 @@ describe('restify middleware - integration test', () => {
annotations.forEach(ann => expect(ann.traceId.spanId).to.equal('bbb'));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -96,7 +96,7 @@ describe('restify middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = restify.createServer();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.post('/foo', (req, res, next) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -140,7 +140,7 @@ describe('restify middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = restify.createServer();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.post('/foo', (req, res, next) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('restify middleware - integration test', () => {
annotations.forEach(ann => expect(ann.traceId.spanId).to.equal('bbb'));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -217,7 +217,7 @@ describe('express middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.post('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -246,7 +246,7 @@ describe('express middleware - integration test', () => {
annotations.forEach(ann => expect(ann.traceId.spanId).to.equal('bbb'));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -284,7 +284,7 @@ describe('express middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.post('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -327,7 +327,7 @@ describe('express middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.post('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -355,7 +355,7 @@ describe('express middleware - integration test', () => {
annotations.forEach(ann => expect(ann.traceId.spanId).to.equal('bbb'));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -399,7 +399,7 @@ describe('connect middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = connect();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.use('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -432,7 +432,7 @@ describe('connect middleware - integration test', () => {
annotations.forEach(ann => expect(ann.traceId.spanId).to.equal('bbb'));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -470,7 +470,7 @@ describe('connect middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = connect();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.use('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -514,7 +514,7 @@ describe('connect middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = connect();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.use('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -547,7 +547,7 @@ describe('connect middleware - integration test', () => {
annotations.forEach(ann => expect(ann.traceId.spanId).to.equal('bbb'));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -595,7 +595,7 @@ describe('connect middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = connect();
app.use(middleware({tracer, serviceName}));
app.use(middleware({tracer}));
app.use('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ describe('CujoJS/rest instrumentation - integration test', () => {

beforeEach(() => {
spans = [];
tracer = new Tracer({ctxImpl: new ExplicitContext(), recorder: newSpanRecorder(spans)});
tracer = new Tracer({
ctxImpl: new ExplicitContext(),
localServiceName: serviceName,
recorder: newSpanRecorder(spans)
});
});

function popSpan() {
Expand All @@ -48,7 +52,7 @@ describe('CujoJS/rest instrumentation - integration test', () => {
}

function getClient() {
return rest.wrap(restInterceptor, {tracer, serviceName, remoteServiceName});
return rest.wrap(restInterceptor, {tracer, remoteServiceName});
}

function url(path) {
Expand Down
14 changes: 6 additions & 8 deletions packages/zipkin-instrumentation-express/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@ const proxy = require('express-http-proxy');

const ctxImpl = new ExplicitContext();
const recorder = new ConsoleRecorder();
const tracer = new Tracer({ctxImpl, recorder});
const serviceName = 'weather-app';
const tracer = new Tracer({ctxImpl, localServiceName: 'weather-app', recorder});
const remoteServiceName = 'weather-api';

const zipkinProxy = wrapExpressHttpProxy(proxy, {tracer, serviceName, remoteServiceName});
const zipkinProxy = wrapExpressHttpProxy(proxy, {tracer, remoteServiceName});

app.use('/api/weather', zipkinProxy('http://api.weather.com', {
decorateRequest: (proxyReq, originalReq) => proxyReq.method = 'POST' // You can use express-http-proxy options as usual
Expand All @@ -50,11 +49,10 @@ const proxy = require('express-http-proxy');

const ctxImpl = new CLSContext();
const recorder = new ConsoleRecorder();
const tracer = new Tracer({ctxImpl, recorder});
const serviceName = 'weather-app';
const tracer = new Tracer({ctxImpl, localServiceName: 'weather-app', recorder});
const remoteServiceName = 'weather-api';

const zipkinProxy = wrapExpressHttpProxy(proxy, {tracer, serviceName, remoteServiceName});
const zipkinProxy = wrapExpressHttpProxy(proxy, {tracer, remoteServiceName});

app.use('/api/weather', expressMiddleware({tracer, serviceName}), zipkinProxy('http://api.weather.com'));
```
app.use('/api/weather', expressMiddleware({tracer}), zipkinProxy('http://api.weather.com'));
```
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function getPathnameFromPath(path) {
}

class ExpressHttpProxyInstrumentation {
constructor({tracer, serviceName, remoteServiceName}) {
constructor({tracer, serviceName = tracer.localEndpoint.serviceName, remoteServiceName}) {
this.tracer = tracer;
this.serviceName = serviceName;
this.remoteServiceName = remoteServiceName;
Expand Down Expand Up @@ -79,11 +79,8 @@ function wrapProxy(proxy, {tracer, serviceName, remoteServiceName}) {
};
}

const instrumentation = new ExpressHttpProxyInstrumentation({
tracer,
serviceName,
remoteServiceName
});
const instrumentation =
new ExpressHttpProxyInstrumentation({tracer, serviceName, remoteServiceName});

const wrappedOptions = options;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,17 @@ const express = require('express');
const middleware = require('../src/expressMiddleware');

describe('express middleware - integration test', () => {
const serviceName = 'weather-app';

it('should record request & response annotations', done => {
const record = sinon.spy();
const recorder = {record};
const ctxImpl = new ExplicitContext();
const tracer = new Tracer({recorder, ctxImpl});
const tracer = new Tracer({recorder, localServiceName: serviceName, ctxImpl});

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({
tracer,
serviceName: 'service-a'
}));
app.use(middleware({tracer}));
app.post('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -50,7 +49,7 @@ describe('express middleware - integration test', () => {
.to.equal(originalSpanId));

expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
expect(annotations[0].annotation.serviceName).to.equal('service-a');
expect(annotations[0].annotation.serviceName).to.equal(serviceName);

expect(annotations[1].annotation.annotationType).to.equal('Rpc');
expect(annotations[1].annotation.name).to.equal('POST');
Expand Down Expand Up @@ -93,10 +92,7 @@ describe('express middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({
tracer,
serviceName: 'service-a'
}));
app.use(middleware({tracer}));
app.get('/foo', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
const ctx = ctxImpl.getContext();
Expand Down Expand Up @@ -138,11 +134,11 @@ describe('express middleware - integration test', () => {
const record = sinon.spy();
const recorder = {record};
const ctxImpl = new ExplicitContext();
const tracer = new Tracer({recorder, ctxImpl});
const tracer = new Tracer({recorder, localServiceName: serviceName, ctxImpl});

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({tracer, serviceName: 'service-a'}));
app.use(middleware({tracer}));

app.get('/foo/:id', (req, res) => {
// Use setTimeout to test that the trace context is propagated into the callback
Expand Down Expand Up @@ -185,7 +181,7 @@ describe('express middleware - integration test', () => {
const record = sinon.spy();
const recorder = {record};
const ctxImpl = new ExplicitContext();
const tracer = new Tracer({recorder, ctxImpl});
const tracer = new Tracer({recorder, localServiceName: serviceName, ctxImpl});

function step(num) {
return new Promise((resolve) => {
Expand All @@ -200,10 +196,7 @@ describe('express middleware - integration test', () => {

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({
tracer,
serviceName: 'service-a'
}));
app.use(middleware({tracer}));

app.get('/foo', (req, res) => step(1)
.then(() => step(2))
Expand Down Expand Up @@ -270,11 +263,11 @@ describe('express middleware - integration test', () => {
const record = sinon.spy();
const recorder = {record};
const ctxImpl = new ExplicitContext();
const tracer = new Tracer({recorder, ctxImpl});
const tracer = new Tracer({recorder, localServiceName: serviceName, ctxImpl});

ctxImpl.scoped(() => {
const app = express();
app.use(middleware({tracer, serviceName: 'service-a'}));
app.use(middleware({tracer}));

app.get('/error', (req, res) => {
tracer.recordBinary('message', 'hello from within app');
Expand Down
Loading