Add the rest of V2 factories and interfaces #626
Conversation
tigrannajaryan
requested review from
bogdandrutu,
flands,
owais,
pjanotti,
rghetia and
songy23
as code owners
Codecov Report
@@ Coverage Diff @@
## master #626 +/- ##
=======================================
Coverage 74.63% 74.64%
=======================================
Files 145 145
Lines 10208 10209 +1
=======================================
+ Hits 7619 7620 +1
Misses 2223 2223
Partials 366 366
Continue to review full report at Codecov.
|
tigrannajaryan
force-pushed
the
feature/tigran/factory2-metric
branch
from
19899c8
to
d362a2b
Compare
tigrannajaryan
force-pushed
the
feature/tigran/factory2-metric
branch
2 times, most recently
from
6192018
to
d797833
Compare
- Added V2 metric factories and interfaces for receivers and exporters. - Added V2 factories and interfaces for processors. - Creation methods in V2 factories accept a CreationParams struct to allow for easier addition of new parameters in the future without breaking the interface. All changes are uniform with previously introduced initial batch of V2 factories and interfaces (except introduction of CreationParams). Issue: open-telemetry#478
tigrannajaryan
force-pushed
the
feature/tigran/factory2-metric
branch
from
d797833
to
763713b
Compare
| @@ -56,9 +64,12 @@ type FactoryV2 interface { | |||
| // CreateTraceReceiverV2 creates a trace receiver based on this config. | |||
| // If the receiver type does not support tracing or if the config is not valid | |||
| // error will be returned instead. | |||
| CreateTraceExporterV2(logger *zap.Logger, cfg configmodels.Exporter) (TraceExporterV2, error) | |||
| CreateTraceExporterV2(ctx context.Context, params CreationParams, cfg configmodels.Exporter) (TraceExporterV2, error) | |||
There was a problem hiding this comment.
Sorry, this didn't occur to me until just now. Do we need V2 in these names like CreateTraceExporterV2? Since the type signatures are different I think that is enough to distinguish Factory and FactoryV2 by keeping CreateTraceExporter with just different parameters. From some quick testing this seems to work.
If for some reason we want components to be explicit about their version we could have the factory have a Version() method but doesn't look like that's needed yet.
There was a problem hiding this comment.
In that case it will make impossible for a factory to implement both interfaces, which may be desirable during transition period. Let me think a bit.
Side note: I cannot find the relevant part of Golang spec that describes interface method matching and thus making what you propose compliant with the spec - do you know where is it formally defined?
There was a problem hiding this comment.
I suggest that we merge as is to avoid more delays and I will look into this separately.
|
Merging as is. I will look into renaming some functions additionally. |
* Add check to sum transform for unknown NumberKind * Initial batching * Move CheckpointSet transform to internal package * Add tests for the Exporter Export method Check batching and general output exporter ResourceMetrics are correct. * Check errors in tests * Apply suggestions from code review Co-Authored-By: Krzesimir Nowak <qdlacz@gmail.com> * Use var instead of multiple calls for group IDs * Fix otlp metric test reporting Co-authored-by: Krzesimir Nowak <qdlacz@gmail.com>
…y#626) Bumps [boto3](https://github.com/boto/boto3) from 1.18.20 to 1.18.21. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](boto/boto3@1.18.20...1.18.21) --- updated-dependencies: - dependency-name: boto3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
All changes are uniform with previously introduced initial batch of
V2 factories and interfaces.
Issue: #478