Change NewSplitDriver paramater and initialization #1798
Conversation
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
bryan-aguilar
requested review from
Aneurysm9,
dashpole,
evantorrie,
jmacd,
MrAlias,
paivagustavo and
XSAM
as code owners
Codecov Report
@@ Coverage Diff @@
## main #1798 +/- ##
=====================================
Coverage 78.6% 78.6%
=====================================
Files 137 137
Lines 7288 7298 +10
=====================================
+ Hits 5729 5739 +10
Misses 1315 1315
Partials 244 244
|
exporters/otlp/otlp_test.go
Outdated
|
|
||
| recordCount := 5 | ||
| assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector())) | ||
| assert.NoError(t, driver.ExportTraces(ctx, nil)) |
There was a problem hiding this comment.
I know this makes no actual difference in the code, but could we change these nils to actual snapshots like the test above?
This is just to make sure future iterations of this can't shortcut nil or 0 len inputs and still pass but fail with real data.
There was a problem hiding this comment.
It would make sense to just abstract this part of the repeated code into a separate function that can be called and will ensure this uniform call.
There was a problem hiding this comment.
That makes a lot of sense. I will work on refining the test cases, was trying to get the core change out for review. Do you want me to change this to a draft PR @MrAlias ?
There was a problem hiding this comment.
@bryan-aguilar if you think the changes needed will be greater than just the two tests here doing in another PR sounds reasonable, but if not including the changes here seems valid.
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
exporters/otlp/otlp_test.go
Outdated
|
|
||
| recordCount := 5 | ||
| assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector())) | ||
| assert.NoError(t, driver.ExportTraces(ctx, nil)) |
There was a problem hiding this comment.
It would make sense to just abstract this part of the repeated code into a separate function that can be called and will ensure this uniform call.
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
MrAlias
added
pkg:exporter:otlp
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
This PR updates the way that NewSplitDriver is initialized and the arguments that it takes. As discussed in the SIG meeting on 2021-04-01 the program should not panic if a user fails to initialize all drivers in the config. NewSplitDriver now accepts a set of options which moves it to be in line with the rest of the code base. A noopDriver is internally created for uninitialized fields.
Resolves #1583