-
Notifications
You must be signed in to change notification settings - Fork 810
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
feat!: move serialization to @opentelemetry/otlp-transformer
#4542
Changes from 23 commits
fc9cb4c
dfffc23
79256ed
76c4667
c14bc9b
432cb41
b98c061
5ebc27c
12c2ad2
ac2d1bd
bb88491
0ff7bf6
9a9c346
5205cae
05640f9
85c3f1e
b9bdeea
fd77885
105da17
9cb2593
4f80ada
e8adc4d
998cf96
07c7170
62cc3b3
1324071
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
[submodule "experimental/packages/otlp-grpc-exporter-base/protos"] | ||
path = experimental/packages/otlp-grpc-exporter-base/protos | ||
url = https://github.com/open-telemetry/opentelemetry-proto.git | ||
[submodule "experimental/packages/otlp-proto-exporter-base/protos"] | ||
path = experimental/packages/otlp-proto-exporter-base/protos | ||
url = https://github.com/open-telemetry/opentelemetry-proto.git | ||
[submodule "experimental/packages/otlp-transformer/protos"] | ||
path = experimental/packages/otlp-transformer/protos | ||
url = https://github.com/open-telemetry/opentelemetry-proto.git |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -193,8 +193,6 @@ describe('OTLPLogExporter - node with proto over http', () => { | |
}); | ||
|
||
it('should open the connection', done => { | ||
collectorExporter.export(logs, () => {}); | ||
|
||
sinon.stub(http, 'request').callsFake((options: any, cb: any) => { | ||
assert.strictEqual(options.hostname, 'foo.bar.com'); | ||
assert.strictEqual(options.method, 'POST'); | ||
|
@@ -206,11 +204,10 @@ describe('OTLPLogExporter - node with proto over http', () => { | |
done(); | ||
return fakeRequest as any; | ||
}); | ||
collectorExporter.export(logs, () => {}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not familiar with these tests. Is it necessary to move this call at the bottom of the test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a good question - I should've put a note here as it's quite odd 🙂 I removed the From what I can tell the In follow-ups, I'll re-structure the transport code in the exporters to use the code from #4415. This will then actually implement the lazy-loading that was intended to be there in the first place. |
||
}); | ||
|
||
it('should set custom headers', done => { | ||
collectorExporter.export(logs, () => {}); | ||
|
||
sinon.stub(http, 'request').callsFake((options: any, cb: any) => { | ||
assert.strictEqual(options.headers['foo'], 'bar'); | ||
|
||
|
@@ -220,11 +217,10 @@ describe('OTLPLogExporter - node with proto over http', () => { | |
done(); | ||
return fakeRequest as any; | ||
}); | ||
collectorExporter.export(logs, () => {}); | ||
}); | ||
|
||
it('should have keep alive and keepAliveMsecs option set', done => { | ||
collectorExporter.export(logs, () => {}); | ||
|
||
sinon.stub(http, 'request').callsFake((options: any, cb: any) => { | ||
assert.strictEqual(options.agent.keepAlive, true); | ||
assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); | ||
|
@@ -235,6 +231,7 @@ describe('OTLPLogExporter - node with proto over http', () => { | |
done(); | ||
return fakeRequest as any; | ||
}); | ||
collectorExporter.export(logs, () => {}); | ||
}); | ||
|
||
it('should successfully send the logs', done => { | ||
|
@@ -271,35 +268,35 @@ describe('OTLPLogExporter - node with proto over http', () => { | |
// Need to stub/spy on the underlying logger as the "diag" instance is global | ||
const spyLoggerError = sinon.stub(diag, 'error'); | ||
|
||
collectorExporter.export(logs, result => { | ||
assert.strictEqual(result.code, ExportResultCode.SUCCESS); | ||
assert.strictEqual(spyLoggerError.args.length, 0); | ||
done(); | ||
}); | ||
|
||
sinon.stub(http, 'request').callsFake((options: any, cb: any) => { | ||
const mockRes = new MockedResponse(200); | ||
cb(mockRes); | ||
mockRes.send('success'); | ||
return fakeRequest as any; | ||
}); | ||
}); | ||
|
||
it('should log the error message', done => { | ||
collectorExporter.export(logs, result => { | ||
assert.strictEqual(result.code, ExportResultCode.FAILED); | ||
// @ts-expect-error verify error code | ||
assert.strictEqual(result.error.code, 400); | ||
assert.strictEqual(result.code, ExportResultCode.SUCCESS); | ||
assert.strictEqual(spyLoggerError.args.length, 0); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('should log the error message', done => { | ||
sinon.stub(http, 'request').callsFake((options: any, cb: any) => { | ||
const mockResError = new MockedResponse(400); | ||
cb(mockResError); | ||
mockResError.send('failed'); | ||
|
||
return fakeRequest as any; | ||
}); | ||
|
||
collectorExporter.export(logs, result => { | ||
assert.strictEqual(result.code, ExportResultCode.FAILED); | ||
// @ts-expect-error verify error code | ||
assert.strictEqual(result.error.code, 400); | ||
done(); | ||
}); | ||
}); | ||
}); | ||
describe('export - with compression', () => { | ||
|
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.
Note: we'll be able to remove this submodule here too in the follow-up to do the same to the browser exporters.