Add exceptions connector

[marctc]

Description: Added connector to create metrics from recorded exceptions. The implementation
is heavily inspired from spanmetricsconnector.

Link to tracking Issue: #17272

Documentation: Added examples in README.

[atoulme]

Sorry, I took this PR out of draft by mistake. I meant to accept the builds to run.

[atoulme]

What does this mean?

[atoulme]

OK, I see how it's used. It might be a good idea to make this standard by formalizing the mapping from trace to metric on opentelemetry-specification. WDYT?

[marctc]

Added a TODO to fix later. Is that enough?

[atoulme]

OK, I see how it's used. It might be a good idea to make this standard by formalizing the mapping from trace to metric on opentelemetry-specification. WDYT?

[marctc]

Hey thanks for review comments! I iniallity opened the PR as draft because still needed to add the part of logs. The PR comments I think are still relevant, I will address them once I'm back from holidays 🙏 .

In the meantime, maybe you could help me with a couple of questions that I have with the connectors. In this branch, I added also support to generate logs in the same connector. Is great because it reduces configuration, but Is there a way to make that same connector shares the same code and process? I initially tried to make everything in the same component but if you don't specify, for example, logs in the service's pipelines the log's consumer will panic when trying to send logs. We don't have a connectorhelper so this requires some code duplication for both signals.

On the other hand, I was also wondering if it's possible that the connector shares the same process when sending metrics and logs. Current implementation relies on a cache where it stores the aggregated observed spans with exceptions to later generate metrics and logs. If someone would want to configure the collector to report exceptions, would have 2 running processes of component with 2 duplicated instances of the cache in memory. Is that something expected?

[jpkrohling]

if you don't specify, for example, logs in the service's pipelines the log's consumer will panic when trying to send logs

It should not panic, I believe. Perhaps @djaglowski can share what's the expectation there.

would have 2 running processes of component with 2 duplicated instances of the cache in memory

This is up to the component. Some components use a singleton pattern, with metrics/traces/logs processors made from a single instance. I believe the same can be accomplished with connectors.

[jpkrohling]

I'll review the actual code later on, but I wanted to mention that this PR seems to be missing a few things that are expected from new components, such as versions.yaml or entries to the internal distribution used for testing.

Here's some information on what's expected from new components: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-new-components

[djaglowski]

It should not panic, I believe. Perhaps @djaglowski can share what's the expectation there.

Correct.

@marctc, If I'm understanding correctly, you tried to implement ConsumeTraces in a way where one call would emit both metrics and logs. You actually need to implement two separate ConsumeTraces functions and trust the collector to call the appropriate one based on the user's configuration.

In other words, consider TracesToMetrics and TracesToLogs to be entirely independent capabilities. The user can use one or the other, or both. You can share code between them, if it makes sense, but don't assume any shared state or try to use one function to emit to both consumers.

There are a couple ways to do this, but here's a straightforward approach. If there's obvious code to share, great. If not, it's ok to just keep everything separate.

// connector.go
type connectorImp struct {
	sharedStuff
}

type tracesToMetricsConnector struct {
	connectorImp
	metricsStuff
	metricsConsumer consumer.Metrics
}

func (tm *tracesToMetricsConnector) ConsumeTraces(ctx context.Context, traces ptrace.Traces) error {
	metrics := doMetricsStuff(traces)
	tm.next.ConsumeMetrics(ctx, metrics)
}

type tracesToLogsConnector struct {
	connectorImp
	logsStuff
	logsConsumer consumer.Logs
}

func (tl *tracesToLogsConnector) ConsumeTraces(ctx context.Context, traces ptrace.Traces) error {
	logs := doLogsStuff(traces)
	tl.next.ConsumeLogs(ctx, logs)
}

// factory.go
func NewFactory() connector.Factory {
	return connector.NewFactory(
		typeStr,
		createDefaultConfig,
		connector.WithTracesToMetrics(createTracesToMetricsConnector, stability),
		connector.WithTracesToLogs(createTracesToLogsConnector, stability),
	)
}

func createTracesToMetricsConnector(ctx context.Context, params connector.CreateSettings, cfg component.Config, nextConsumer consumer.Metrics) (connector.Traces, error) {
	c, err := newConnector(params.Logger, cfg, ...)
	if err != nil {
		return nil, err
	}
	return &tracesToMetricsConnector{
		connectorImp: c,
		metricsStuff: initMetricsStuff(),
		metricsConsumer: nextConsumer,
	}, nil
}

func createTracesToLogsConnector(ctx context.Context, params connector.CreateSettings, cfg component.Config, nextConsumer consumer.Logs) (connector.Traces, error) {
	c, err := newConnector(params.Logger, cfg, ...)
	if err != nil {
		return nil, err
	}
	return &tracesToLogsConnector{
		connectorImp: c,		
		logsStuff: initLogsStuff(),
		logsConsumer: nextConsumer,
	}, nil
}

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[djaglowski]

I'd love to see this move forward. @marctc will you still be able to work on this?

[marctc]

I'd love to see this move forward. @marctc will you still be able to work on this?

Hey Daniel. Yes, I'm actually working on this right now after my vacations. Thanks for the encouragement!

[marctc]

The PR is ready for review, thanks for waiting. I address some of the initial concerns, I hope this goes in the right direction.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[jpkrohling]

Could you please create a tracking issue and mark this as resolved?

[jpkrohling]

I forgot already, are you getting all HTTP attributes, or are you using a fixed list of attributes? It'd be good to be clear here. If it's all HTTP attributes, it would be good to state exactly which semantic convention version(s) you support.

And as a feature request: allow users to specify which semantic convention(s) they want to follow.

[jpkrohling]

This answers my previous question. The readme has to tell users to expect all attributes starting with "http." are going to be copied over.

[marctc]

@jpkrohling I addressed the README feedback. I thin in order to create follow-up issues we have the merge this one first (then I can choose this component in the Component(s) dropdown from Issue: Feature request form). Unless you suggest it doing before hand. Thanks!