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.

Sign up for GitHub

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

Jump to bottom

Added agent option to http transport #439

Merged
merged 8 commits into from
Aug 19, 2019
Merged

Added agent option to http transport #439

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d5fbc55
Added agent option to http transport
RAWeber Aug 12, 2019
2add964
Added type for http transport agent option
RAWeber Aug 13, 2019
4109619
Fix lint imports
RAWeber Aug 13, 2019
9199c5f
Added typescript tests
RAWeber Aug 15, 2019
7a6b616
Fix lint errors
RAWeber Aug 15, 2019
86f4c73
Add URL parameter to Agent function
RAWeber Aug 16, 2019
6e391ba
Update http transport agent test
RAWeber Aug 16, 2019
c85d8c2
Ignore tsr-declarations.js file when linting
RAWeber Aug 16, 2019
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
5 changes: 5 additions & 0 deletions packages/zipkin-transport-http/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import {Agent as HttpAgent } from 'http';
import {Agent as HttpsAgent} from 'https';
import {JsonEncoder, Logger, model} from 'zipkin';

type Agent = HttpAgent | HttpsAgent;

declare class HttpLogger implements Logger {
constructor(options: {
endpoint: string,
httpInterval?: number,
jsonEncoder?: JsonEncoder,
httpTimeout?: number,
headers?: { [name: string]: any },
agent?: Agent | (() => Agent),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is (() => Agent) the same effect as null?

Let's add typescript and javascript tests for this parameter to make sure someone later doesn't accidentally break what you need.

Copy link
Contributor Author

@RAWeber RAWeber Aug 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(() => Agent) is the equivalent of a function which returns an Agent

This follows node-fetch which will accept an HTTP(S) Agent or a function which returns a HTTP(S) Agent

I have also added typescript tests, to ensure the current functionality & also the newly added agent types.

log?: Console
});
logSpan(span: model.Span): void;
Expand Down
3 changes: 3 additions & 0 deletions packages/zipkin-transport-http/src/HttpLogger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class HttpLogger extends EventEmitter {
constructor({
endpoint,
headers = {},
agent = null,
httpInterval = 1000,
jsonEncoder = JSON_V1,
timeout = 0,
Expand All @@ -23,6 +24,7 @@ class HttpLogger extends EventEmitter {
super(); // must be before any reference to *this*
this.log = log;
this.endpoint = endpoint;
this.agent = agent;
this.maxPayloadSize = maxPayloadSize;
this.queue = [];
this.queueBytes = 0;
Expand Down Expand Up @@ -88,6 +90,7 @@ class HttpLogger extends EventEmitter {
body: postBody,
headers: self.headers,
timeout: self.timeout,
agent: self.agent
}).then((response) => {
if (response.status !== 202 && response.status !== 200) {
const err = 'Unexpected response while sending Zipkin data, status:'
Expand Down