Connectors prototype v2 #6372

djaglowski · 2022-10-21T22:19:01Z

2nd iteration of connectors prototype. (Follows #6140)

This PR does the following:

Formalizes the connector component in a way that does not lean on receiver and exporter implementations.
Articulates the possible combinations of signal types, and allows each connector to implement any or all combinations. Also allows each connector to declare a stability level for each combination.
Includes validation that a connector is actually used as a receiver in at least one pipeline and an exporter in at least one.
Changes service/internal/pipelines/Pipelines to an interface with two implementations. The new implementation is available behind a feature gate called: service.enableConnectors
Detects and rejects cycles in the graph.
Builds, starts, and stops all components in the collector according to a topological ordering, determined by the graph based struct.
Add a couple simple connectors to demonstrate capabilities. (nopconnector, countconnector)
Tests are quite robust. There is new test framework for defining and validating pipelines that include connectors.

Here is a sample configuration that uses both the nop and count connectors to accomplish a couple simple tasks:

receivers:
  # Read logs from two files
  filelog/blue:
    include: ./local/blue_green/in/blue.log
    start_at: beginning
  filelog/green:
    include: ./local/blue_green/in/green.log
    start_at: beginning

exporters:
  # Write each log to a corresponding file
  file/logs_blue:
    path: ./local/blue_green/out/logs_blue.jsonl
  file/logs_green:
    path: ./local/blue_green/out/logs_green.jsonl

  # Write a copy of all logs to a single file
  file/logs_all:
    path: ./local/blue_green/out/logs_all.jsonl

  # Count the number of logs from each file
  file/metrics_blue:
    path: ./local/blue_green/out/metrics_blue.jsonl
  file/metrics_green:
    path: ./local/blue_green/out/metrics_green.jsonl

  # Count the number of logs from both files
  file/metrics_all:
    path: ./local/blue_green/out/metrics_all.jsonl

connectors:
  count/blue:
  count/green:
  count/all:
  nop/all:

service:
  pipelines:
    logs/blue:
      receivers: [filelog/blue]
      exporters: [file/logs_blue, nop/all, count/blue, count/all]
    logs/green:
      receivers: [filelog/green]
      exporters: [file/logs_green, nop/all, count/green, count/all]
    logs/all:
      receivers: [nop/all]
      exporters: [file/logs_all]

    metrics/blue:
      receivers: [count/blue]
      exporters: [file/metrics_blue]
    metrics/green:
      receivers: [count/green]
      exporters: [file/metrics_green]
    metrics/all:
      receivers: [count/all]
      exporters: [file/metrics_all]

To run this, make sure to enable the feature gate:

./bin/otelcorecol_darwin_arm64 --config ./local/blue_green/config.yaml --feature-gates=service.enableConnectors

codecov · 2022-10-26T19:20:26Z

Codecov Report

Base: 90.36% // Head: 90.45% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (e7bf133) compared to base (6a2dc30).
Patch coverage: 80.87% of modified lines in pull request are covered.

❗ Current head e7bf133 differs from pull request most recent head 3c41db9. Consider uploading reports for the commit 3c41db9 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6372      +/-   ##
==========================================
+ Coverage   90.36%   90.45%   +0.09%     
==========================================
  Files         244      249       +5     
  Lines       14161    15083     +922     
==========================================
+ Hits        12796    13644     +848     
- Misses       1115     1142      +27     
- Partials      250      297      +47

Impacted Files	Coverage Δ
component/component.go	`100.00% <ø> (+36.17%)`	⬆️
component/componenttest/nop_factories.go	`25.00% <0.00%> (+25.00%)`	⬆️
service/host.go	`100.00% <ø> (ø)`
cmd/otelcorecol/components.go	`62.50% <57.14%> (-1.14%)`	⬇️
service/internal/pipelines/nodes.go	`65.44% <65.44%> (ø)`
cmd/builder/internal/builder/config.go	`68.67% <72.72%> (+0.18%)`	⬆️
service/internal/pipelines/graph.go	`74.08% <74.08%> (ø)`
service/service.go	`70.16% <78.94%> (+0.71%)`	⬆️
connector/nopconnector/nop.go	`82.22% <82.22%> (ø)`
connector/countconnector/count.go	`91.91% <91.91%> (ø)`
... and 101 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

config/connector.go

connector/countconnector/count.go

bogdandrutu · 2022-11-02T14:24:54Z

component/connector.go

+// then process and export the metric to the appropriate backend.
+type MetricsToMetricsConnector interface {
+	Connector
+	ConsumeMetricsToMetrics(ctx context.Context, tm pmetric.Metrics) error


Why do we need new func (ConsumeMetricsToMetrics) and just implementing consumer.Metrics is not enough? Signature is equivalent.

This was my original implementation but I found it to be confusing in some ways. I don't think there's anything that technically prevents us from doing it as you suggested so I can switch it back if you want.

To try to summarize the confusion I found though:

Conceptually, any connector that consumes traces is a consumer.Traces, regardless of what it does with them. We also need each signal combination to implement a different interface, and as you pointed out consumer.Traces can differentiate traces->traces from the other combinations. However, the others ultimately must be wrapped in consumer.Traces as well in order to allow other components to send data to them. I found this confusing because it means that traces->traces is always consumer.Traces, but traces->metrics is only sometimes consumer.Traces, depending on where in the code. I found it easier to just treat all combinations the same so we can say they each have a dedicated interface, and so they all become consumer.Traces at the same time when building the graph.

I'm not certain, but from a component author's point of view, the same confusion may be relevant.

Can you give me a bit of code snippet to make sure we understand. I am a bit confused about the explanation.

I'll try to explain better. It may be necessary to first step back and highlight a couple design questions that I worked through in this implementation.

What constraints should be placed on the input->output types supported by a connector type?

Based on the input/output constraints, how do users reason about configuration?

Based on the input/output constraints, how should connectors be managed within the collector - instanced per-pipeline like processors, shared per-signal like receivers/exporters, or something else?

What constraints should be placed on the input->output types supported by a connector type?

I considered the following options:

A connector type can support exactly one input/output combination. This is too granular and prevents intuitive use cases like nop and count, instead requiring noptraces, nopmetrics, etc.

A connector type can support exactly one output type per input type. This works for nop (traces->traces, metrics->metrics, logs->logs) and count (traces->metrics, metrics->metrics, logs->logs).

A connector type can support any or all input/output combinations. This is the least restrictive but introduces some implementation considerations. Notably though, this. is necessary to directly support behavior like spanmetricsprocessor, which both derives metrics from spans and allows the original spans to pass through unmodified. (i.e.traces->traces, traces->metrics).

I think option 3 is best because it allows for the most use cases / is the least restrictive.

Based on the input/output constraints, how do users reason about configuration?

In short, a connector will do all the things it can do that are applicable to the pipelines it connects.

A connector type may consume one or more signal types and emit one or more signal types.

Each connector type declares support for specific combinations of signal types.

Each signal combination supported by a connector is an independent function. That is, the connector does not require that you use all if its capabilities. For example, if the count connector is configured to connect from a logs pipeline to a metrics pipeline, it will count logs. Likewise, if you do not want the spanmetrics connector to forward traces, you can configure only the traces->metrics connection and ignore the traces->traces capability.

A connector will consume/emit for each pair of pipelines that it is connected.

If a connector configured in such a way that any combination of its input/output pipelines are unsupported, this will result in a configuration error.

Based on the input/output constraints, how should connectors be managed within the collector - instanced per-pipeline like processors, shared per-signal like receivers/exporters, or something else?

A connector is instanced based on each input/output type. This is similar to how receivers and exporters are managed internally. Specially, receivers and exporters are instanced per signal type. (Technically, the component developer can use sharedcomponent to unify these instances, but the collector manages them via a separate handle for each signal type.)

This example shows a connector that is consuming and emitting all three signal types. Internally, the collector will instantiate 9 instances of this connector, with each instance constructed by the corresponding Create<DataType>To<DataType>Connector factory method. Likewise, each instance of the connector will consume signals by performing the corresponding Consume<DataType>To<DataType> function.

In such a case, each of the 9 instances must ultimately satisfy one of the consumer.<DataType> interfaces. This is done by wrapping each instances via the appropriate consumer.New<DataType> function. Here is the code that handles this, but as a simplified version:

switch receiverPipelineType { case component.DataTypeTraces: ... switch exporterPipelineType { case component.DataTypeTraces: connectorInstance := n.factory.CreateTracesToTracesConnector(ctx, set, cfg, fanoutConsumer) tracesConsumer := consumer.NewTraces(connectorInstance.ConsumeTracesToTraces) ... case component.DataTypeMetrics: connectorInstance := n.factory.CreateMetricsToTracesConnector(ctx, set, cfg, fanoutConsumer) metricsConsumer := consumer.NewMetrics(connectorInstance.ConsumeMetricsToTraces) ... case component.DataTypeLogs: connectorInstance := n.factory.CreateLogsToTracesConnector(ctx, set, cfg, fanoutConsumer) logsConsumer := consumer.NewLogs(connectorInstance.ConsumeLogsToTraces) ... }

We could rename ConsumeTracesToTraces to ConsumeTraces, but then it is a special case vs ConsumeMetricsToTraces and ConsumeLogsToTraces. This is harder to keep track of in my opinion. It becomes especially awkward when trying to unwrap the connector instance, such as is necessary in tests when validating that each component in the graph is as expected. By treating each signal combination as equal, it allows for uniform code such as this.

@djaglowski this requires every pairwise combination of signals to have an interface. Is this approach going to scale if we add one or two other new signals? It is 3x3 now, if we add 2 more signals (e.g. profiles and entities), we will have 5x5=25 interfaces, 25 methods and 25 switch cases, etc.
Is there a way to use generics here somehow to reduce the code volume? (Probably no, I don't remember Go's genics having much compile-time power to generate combinations of type parameters).

25 combinations seems... manageable, if far from ideal. The scalability issues is somewhat mitigated in that we likely will not exceed 5 signal types any time soon. I agree though - the current implementation will become quite cumbersome when adding additional signals.

I'll spend some time looking into this. There might be a way to decouple the input/output types so that a connector has to implement one consumer type and one "emitter" type, and then rely on composition rather than pairwise combinations.

I have not been able to identify a good way to do this.

I don't see a way to use generics here, though I may be missing it.

I tried removing the pairwise interfaces and defining one interface per signal type, with the signal type indicating the type of consumer that the connector itself it. This lead to a need to expose a generic consumer.Consumer which each implementation had to type check during connector creation. This feels error prone and I don't see how to avoid it.

I looked into defining a "TracesConnector" as a connector that emits traces, but this feels like a nonstarter as it completely sidesteps the usefulness of each component being a clear type of consumer.

There may be a better way here, but I wonder if we can live with the explicit pairwise code, at least initially.

I'll play around with this a bit, but I believe it might be possible to use generics here to alleviate at least part of the combinatorial problem.

djaglowski

@bogdandrutu, thanks for taking a look.

I responded to your feedback and am continuing to work through tests.

connector/countconnector/count.go

djaglowski · 2022-11-02T15:39:06Z

component/connector.go

+// then process and export the metric to the appropriate backend.
+type MetricsToMetricsConnector interface {
+	Connector
+	ConsumeMetricsToMetrics(ctx context.Context, tm pmetric.Metrics) error


This was my original implementation but I found it to be confusing in some ways. I don't think there's anything that technically prevents us from doing it as you suggested so I can switch it back if you want.

To try to summarize the confusion I found though:

Conceptually, any connector that consumes traces is a consumer.Traces, regardless of what it does with them. We also need each signal combination to implement a different interface, and as you pointed out consumer.Traces can differentiate traces->traces from the other combinations. However, the others ultimately must be wrapped in consumer.Traces as well in order to allow other components to send data to them. I found this confusing because it means that traces->traces is always consumer.Traces, but traces->metrics is only sometimes consumer.Traces, depending on where in the code. I found it easier to just treat all combinations the same so we can say they each have a dedicated interface, and so they all become consumer.Traces at the same time when building the graph.

I'm not certain, but from a component author's point of view, the same confusion may be relevant.

config/connector.go

ahayworth · 2022-11-17T20:40:41Z

You seem to have a better understanding of the feature than you give yourself credit for.

Thank you, that's kind. And I appreciate that you are willing to hear feedback on usability aspects of it, which I know can be tedious at times (but so important to get right).

The lists of connectors, exporters, receivers are easy enough to refer back to if you don't recall immediately. In the example you cited, it's just a few lines above the pipelines section

I do so wish that collector configs I work with would fit on one (or even two) pages in my editor. 😂

But I do see the point you're making. I will concede that perhaps my usability worries are not representative of the what the average person may be working with!

All that said, if there were a clear way to improve this, we can consider it.

Reading through the rest of your comment and the subsequent ones with the options considered: perhaps this is the best way after all. I appreciate that you walked us through all the options so thoroughly, it really helps me to see that this may not be ideal but is a lot better than many feasible alternatives.

I'll keep thinking about configuration usability in general, and if I come up with anything maybe we can look at that as a general improvement to make for the collector overall. :)

djaglowski · 2022-11-18T17:39:00Z

I've pulled out the skeleton of the new component type for review here: #6577

tigrannajaryan · 2022-11-18T18:12:10Z

@djaglowski a slight modification of what you suggested:

connectors:
  count/1: # converts telemetry to metrics by counting the number of records

service:
  pipelines:
    logs/blue:
      receivers: [filelog/blue]
      exporters: [file/logs_blue]
      connect_to: 
         - pipeline: logs/all # forward from logs/blue pipeline to logs/all pipeline the data as is.
    logs/green:
      receivers: [filelog/green]
      exporters: [file/logs_green]
      connect_to:
        - pipeline: metrics/all # forward from logs/green pipeline to metrics/all pipeline
          via: count/1 # use this connector logic. If omitted uses "forward" ("noop") connector.
    logs/all:
      exporters: [file/logs_all]
    metrics/all:
       ...

With this approach there is no need to refer to the same connector twice, we only refer to it in the source pipeline.

djaglowski · 2022-11-18T19:45:26Z

connectors:
  count/1: # converts telemetry to metrics by counting the number of records

service:
  pipelines:
    logs/blue:
      receivers: [filelog/blue]
      exporters: [file/logs_blue]
      connect_to: 
         - pipeline: logs/all # forward from logs/blue pipeline to logs/all pipeline the data as is.
    logs/green:
      receivers: [filelog/green]
      exporters: [file/logs_green]
      connect_to:
        - pipeline: metrics/all # forward from logs/green pipeline to metrics/all pipeline
          via: count/1 # use this connector logic. If omitted uses "forward" ("noop") connector.
    logs/all:
      exporters: [file/logs_all]
    metrics/all:
       ...

With this approach there is no need to refer to the same connector twice, we only refer to it in the source pipeline.

Is it really a problem to refer to a connector twice? What is the goal in preventing this?

To me, this approach appears to be more verbose anyways, and more importantly I think it blurs some concepts about the pipeline/component model that users already understand.

At the core of our pipeline model is a very simple rule, that data enters via receivers and exits via exporters. This presentation conveys that we are sending data "from a pipeline, to a pipeline" and immediately raises questions about whether there is an exporter or receiver involved. (Of course there is, but shouldn't this be apparent to users?)

Arguably this presentation also challenges the notion that, while a pipeline is an arrangement of components, it is the components themselves that handle the data. If connectors are handling data, aren't they necessarily components? Isn't it then more intuitive to clearly represent connectors as components that are inside of pipelines?

I think we need to decide how we want users to understand connectors. Should they understand that a connector is a component and therefore exists inside of pipelines, as they understand with receivers, processors, and exporters? Or, do we want them to adopt a mental model where connectors are a special concept with an ambiguous relationship to the component/pipeline model? I personally have a strong preference for the former, and I think that the original design does the best job of conveying this model to the user.

tigrannajaryan · 2022-11-21T15:09:52Z

Is it really a problem to refer to a connector twice? What is the goal in preventing this?

It seems slightly less verbose, particularly when using the "nop" connector. The downside is it is more "asymmetric", I somehow like the symmetry in your proposal.

I personally have a strong preference for the former, and I think that the original design does the best job of conveying this model to the user.

I am OK with your proposal. I do not think what I suggested is much better.

djaglowski · 2022-11-29T02:05:40Z

component/factories.go

@@ -32,6 +32,9 @@ type Factories struct {

 	// Extensions maps extension type names in the config to the respective factory.
 	Extensions map[Type]ExtensionFactory
+
+	// Connectors maps connector type names in the config to the respective factory.
+	Connectors map[Type]ConnectorFactory


Since the factory was added to connectors package in #6611, this would ideally be changed:

Suggested change

Connectors map[Type]ConnectorFactory

Connectors map[Type]connector.Factory

However, this causes a circular dependency.

@bogdandrutu, is the plan still to move Factories to the service package, similar to #6552?

Yes :) You can create a PR for that, would be great. Start with a prototype (just the public api changes) so I can give feedback and not feel bad that you have to re-work things :)

I see some work was done on this. For my part, I had taken a crack at it but hadn't worked out how to avoid circular dependency problems.

This branch is finally rebased after the removal of service/internal/pipelines, so I will take another look at this soon.

jpkrohling · 2022-11-29T20:30:11Z

Just wanted to mention that this is high on my list to review :-)

jpkrohling · 2022-11-30T16:37:19Z

While I love the idea of having a native way to convert signals, I still couldn't think of a use case for the pure connection of pipelines that today's features wouldn't cover. Concretely, I don't understand the advantage of this:

    logs/all:
      receivers: [nop/all]
      exporters: [file/logs_all]

over this:

    logs/all:
      receivers: [filelog/blue, filelog/green]
      exporters: [file/logs_all]

djaglowski · 2022-11-30T17:23:17Z

While I love the idea of having a native way to convert signals, I still couldn't think of a use case for the pure connection of pipelines that today's features wouldn't cover. Concretely, I don't understand the advantage of this:
    logs/all:
      receivers: [nop/all]
      exporters: [file/logs_all]
over this:
    logs/all:
      receivers: [filelog/blue, filelog/green]
      exporters: [file/logs_all]

Adding some processors can illustrate the usefulness of same-signal connectors. Below, suppose we need to pre-process the two files in different ways, and then subsequently we wish to batch all the resulting logs together. (I'll call the connector forward as discussed elsewhere, but it is the same as nop.)

service:
  pipelines:
    logs/blue:
      receivers: [filelog/blue]
      processors: [transform/blue] # something particular to blue
      exporters: [backend/blue, forward]
    logs/green:
      receivers: [filelog/green]
      processors: [transform/green] # something particular to green
      exporters: [backend/green, forward]
    logs/all:
      receivers: [forward]
      processors: [batch, transform/common]
      exporters: [backend/all]

Another case based only on forward - you may wish to replicate a signal for use in two different ways.

service:
  pipelines:
    logs:
      receivers: [filelog]
      processor: [transform]
      exporters: [forward]
    logs/all:
      receivers: [forward]
      exporters: [low_cost_backend]
    logs/filtered:
      receivers: [forward]
      processors: [filter]
      exporters: [expensive_backend]

Arguably, the routing processor would be a good use case for a same-signal connector. As a connector, we can do more than just immediately export the data.

service:
  pipelines:
    logs:
      receivers: [filelog]
      exporters: [routing] # routes to one pipeline or the other based on some condition
    logs/type1:
      receivers: [routing]
      processors: [transform/type1]
      exporters: [backend]
    logs/type2:
      receivers: [forward]
      processors: [transform/type2]
      exporters: [backend]

Another example is the count connector. Technically, emitting a count of metrics could be done within a processor, but then you've mixed the derived signal with the original data stream. The user may wish to emit to a different backend instead, or process the signal differently. Additionally, if we would have such a connector for counting logs and spans, it would be intuitive to also have it count metrics.

service:
  pipelines:
    metrics/in:
      receivers: [otlp]
      exporters: [nop, count]
    metrics/out:
      receivers: [nop]
      processors: [transform] # something particular to the original signals
      exporters: [backend]
    metric/counts:
      receivers: [count]
      exporters: [backend/counts_only]

jpkrohling

I didn't finish the review yet but I wanted to release the comments before I leave for the day.

jpkrohling · 2022-11-30T18:00:32Z

cmd/otelcorecol/builder-config.yaml

@@ -17,11 +17,18 @@ extensions:
 processors:
  - gomod: go.opentelemetry.io/collector/processor/batchprocessor v0.65.0
  - gomod: go.opentelemetry.io/collector/processor/memorylimiterprocessor v0.65.0
+connectors:
+  - import: go.opentelemetry.io/collector/connector/countconnector
+    gomod: go.opentelemetry.io/collector/connector/countconnector v0.0.0


Can it be v0.65.0, given that it will be replaced anyway?

jpkrohling · 2022-11-30T18:03:11Z

component/componenttest/nop_connector.go

+				ConnectorSettings: config.NewConnectorSettings(component.NewID("nop")),
+			}
+		},
+		component.WithTracesToTracesConnector(createTracesToTracesConnector, component.StabilityLevelStable),


It can't be stable yet, from what I remember. It should be Alpha.

Good catch. Not intended.

jpkrohling · 2022-11-30T18:10:27Z

component/connector.go

+
+var (
+	errDataTypesNotSupported = "connection from %s to %s is not supported"
+	ErrTracesToTraces        = fmt.Errorf(errDataTypesNotSupported, DataTypeTraces, DataTypeTraces)


It might be clearer later on during the review, but should those be part of the public API?

They are exported temporarily because the component package tests are technically in another package component_test.

jpkrohling · 2022-11-30T18:33:23Z

component/connector.go

+// then process and export the metric to the appropriate backend.
+type MetricsToMetricsConnector interface {
+	Connector
+	ConsumeMetricsToMetrics(ctx context.Context, tm pmetric.Metrics) error


I'll play around with this a bit, but I believe it might be possible to use generics here to alleviate at least part of the combinatorial problem.

jpkrohling · 2022-12-05T16:02:00Z

connector/countconnector/count.go

+	})
+
+	conn := comp.Unwrap().(*countConnector)
+	conn.metricsConsumer = nextConsumer


Perhaps I'm missing something, but if you have the same connector for different input signals and different output signals, won't the last output override the previous ones? Like:

logs -> connector/1 -> metrics/prometheus1
traces -> connector/1 -> metrics/prometheus2

In the current code, won't this end up with logs sending data to prometheus2?

In this case, each instantiation of the connector would emit to both prometheus1 and prometheus2 because the connector is acting as a receiver in a pipeline with two exporters, or a receiver in two pipelines. Either way, the data should be fanned out to both. The service package is responsible for detecting this relationship during build time and wrapping consumers into a fanoutconsumer when appropriate.

jpkrohling · 2022-12-05T16:03:02Z

connector/countconnector/count.go

+// it should be removed from this map so the same configuration can be recreated successfully.
+var connectors = sharedcomponent.NewSharedComponents()
+
+// otlpReceiver is the type that exposes Trace and Metrics reception.


Suggested change

// otlpReceiver is the type that exposes Trace and Metrics reception.

// countConnector counts the input data points, converting to metrics

jpkrohling · 2022-12-05T18:30:55Z

connector/countconnector/go.mod

@@ -0,0 +1,55 @@
+module go.opentelemetry.io/collector/connector/countconnector
+
+go 1.19


Before merging, can you ensure this is aligned with the main go.mod? I think we are still on 1.18

jpkrohling · 2022-12-05T18:31:09Z

connector/countconnector/go.mod

+
+require (
+	github.com/stretchr/testify v1.8.1
+	go.opentelemetry.io/collector v0.65.0


Same for these :-)

jpkrohling · 2022-12-05T18:32:06Z

connector/nopconnector/README.md

+The `nopconnector` can be used to chain pipelines of the same type together.
+For example, it can replicate a signal to multiple pipelines so that each pipeline
+can process the signal independently in varying ways. Alternately, it can be used to
+merge two pipelines together so they can be processed as one pipeline.


A paragraph here with a couple of use-cases would be nice to have. I myself wouldn't know why I would need this.

jpkrohling · 2022-12-05T18:38:08Z

service/config.go

 			}
+			if cfg.Connectors[ref] != nil {


Same here:

Suggested change

if cfg.Connectors[ref] != nil {

if _, ok := cfg.Connectors[ref]; ok {

jpkrohling · 2022-12-05T18:39:18Z

service/config.go

@@ -124,9 +145,14 @@ func (cfg *Config) validateService() error {
 		// Validate pipeline receiver name references.
 		for _, ref := range pipeline.Receivers {
 			// Check that the name referenced in the pipeline's receivers exists in the top-level receivers.
-			if cfg.Receivers[ref] == nil {
-				return fmt.Errorf("pipeline %q references receiver %q which does not exist", pipelineID, ref)
+			if cfg.Receivers[ref] != nil {


Is there a performance gain if you use the optional second return value (bool) from the map lookup?

Suggested change

if cfg.Receivers[ref] != nil {

if _, ok := cfg.Receivers[ref]; ok {

Good point. I think your suggestion is more idiomatic either way.

jpkrohling · 2022-12-05T18:39:44Z

service/config.go

@@ -152,15 +178,33 @@ func (cfg *Config) validateService() error {
 		// Validate pipeline exporter name references.
 		for _, ref := range pipeline.Exporters {
 			// Check that the name referenced in the pipeline's Exporters exists in the top-level Exporters.
-			if cfg.Exporters[ref] == nil {
-				return fmt.Errorf("pipeline %q references exporter %q which does not exist", pipelineID, ref)
+			if cfg.Exporters[ref] != nil {


And here?

Suggested change

if cfg.Exporters[ref] != nil {

if _, ok := cfg.Exporters[ref]; ok {

jpkrohling · 2022-12-05T18:40:49Z

service/config.go

+			if cfg.Exporters[ref] != nil {
+				continue
+			}
+			if cfg.Connectors[ref] != nil {


Here as well:

Suggested change

if cfg.Connectors[ref] != nil {

if _, ok := cfg.Connectors[ref]; ok {

djaglowski · 2022-12-12T20:27:59Z

Closing in favor of #6700

djaglowski force-pushed the connectors-prototype-v2 branch 2 times, most recently from af859b6 to 001d0d2 Compare October 26, 2022 19:12

djaglowski force-pushed the connectors-prototype-v2 branch 9 times, most recently from 5021741 to ca14718 Compare November 2, 2022 13:39

bogdandrutu reviewed Nov 2, 2022

View reviewed changes

config/connector.go Outdated Show resolved Hide resolved

connector/countconnector/count.go Show resolved Hide resolved

bogdandrutu reviewed Nov 2, 2022

View reviewed changes

djaglowski commented Nov 2, 2022

View reviewed changes

djaglowski force-pushed the connectors-prototype-v2 branch 15 times, most recently from e6021ef to 244af4c Compare November 8, 2022 15:29

djaglowski mentioned this pull request Nov 18, 2022

Define new component type "connector" #6577

Merged

djaglowski force-pushed the connectors-prototype-v2 branch 3 times, most recently from 4b5bac6 to f44b022 Compare November 22, 2022 21:16

newly12 mentioned this pull request Nov 23, 2022

Add filereload receiver open-telemetry/opentelemetry-collector-contrib#15660

Closed

djaglowski force-pushed the connectors-prototype-v2 branch from f44b022 to 67b2867 Compare November 29, 2022 02:01

djaglowski commented Nov 29, 2022

View reviewed changes

djaglowski force-pushed the connectors-prototype-v2 branch from 67b2867 to d6d8bd6 Compare November 29, 2022 02:15

jpkrohling self-requested a review November 29, 2022 20:30

jpkrohling reviewed Nov 30, 2022

View reviewed changes

jpkrohling reviewed Dec 5, 2022

View reviewed changes

djaglowski force-pushed the connectors-prototype-v2 branch from d6d8bd6 to 846b039 Compare December 5, 2022 22:15

djaglowski mentioned this pull request Dec 5, 2022

Fix some pipeline tests and validate error messages #6685

Merged

djaglowski force-pushed the connectors-prototype-v2 branch 2 times, most recently from e733098 to 53f526b Compare December 6, 2022 17:30

djaglowski mentioned this pull request Dec 6, 2022

Add connector types #6689

Merged

Prototype "connectors", a new type of pipeline component

3c41db9

djaglowski force-pushed the connectors-prototype-v2 branch from 53f526b to 3c41db9 Compare December 7, 2022 20:20

djaglowski mentioned this pull request Dec 7, 2022

Connectors prototype v3 #6700

Closed

djaglowski closed this Dec 12, 2022

	Connectors map[Type]ConnectorFactory
	Connectors map[Type]connector.Factory

	// otlpReceiver is the type that exposes Trace and Metrics reception.
	// countConnector counts the input data points, converting to metrics

		@@ -0,0 +1,55 @@
		module go.opentelemetry.io/collector/connector/countconnector

		go 1.19

	if cfg.Connectors[ref] != nil {
	if _, ok := cfg.Connectors[ref]; ok {

	if cfg.Receivers[ref] != nil {
	if _, ok := cfg.Receivers[ref]; ok {

	if cfg.Exporters[ref] != nil {
	if _, ok := cfg.Exporters[ref]; ok {

Connectors prototype v2 #6372

Connectors prototype v2 #6372

Conversation

djaglowski commented Oct 21, 2022 • edited Loading

codecov bot commented Oct 26, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

djaglowski Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djaglowski left a comment

Choose a reason for hiding this comment

djaglowski Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

ahayworth commented Nov 17, 2022

djaglowski commented Nov 18, 2022

tigrannajaryan commented Nov 18, 2022

djaglowski commented Nov 18, 2022

tigrannajaryan commented Nov 21, 2022

Choose a reason for hiding this comment

bogdandrutu Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpkrohling commented Nov 29, 2022

jpkrohling commented Nov 30, 2022

djaglowski commented Nov 30, 2022 • edited Loading

jpkrohling left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djaglowski Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djaglowski Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djaglowski commented Dec 12, 2022

djaglowski commented Oct 21, 2022 •

edited

Loading

codecov bot commented Oct 26, 2022 •

edited

Loading

djaglowski Nov 2, 2022 •

edited

Loading

djaglowski Nov 2, 2022 •

edited

Loading

bogdandrutu Nov 29, 2022 •

edited

Loading

djaglowski commented Nov 30, 2022 •

edited

Loading

djaglowski Dec 5, 2022 •

edited

Loading

djaglowski Dec 5, 2022 •

edited

Loading