[receiver/faro] add initial implementation of faroreceiver #19183

rlankfo · 2023-03-03T01:47:32Z

Description:
This adds an initial implementation of a Faro receiver.

Link to tracking Issue: #19180

Testing:
Ran the collector in local distribution.

Documentation:

Signed-off-by: Robbie Lankford <robert.lankford@grafana.com>

receiver/faroreceiver/README.md

kovrus · 2023-03-03T10:38:18Z

receiver/faroreceiver/faro.go

+func (r *faroReceiver) Start(ctx context.Context, host component.Host) error {
+	var err error
+	r.startOnce.Do(func() {
+		if r.cfg.HTTP != nil {


HTTP server won't be started and users won't be notified about it. Let's maybe validate the configuration by implementing the component.ConfigValidator interface for Config.

kovrus · 2023-03-03T10:42:42Z

receiver/faroreceiver/handler.go

+	}
+
+	if p.Traces != nil && p.Traces.SpanCount() > 0 {
+		err = h.traceConsumer.ConsumeTraces(ctx, p.Traces.Traces)


Maybe we can use multierr here, so if both ConsumeLogs and ConsumeTraces fail, the error from the former won't be overwritten

kovrus · 2023-03-03T10:55:18Z

receiver/faroreceiver/handler.go

+
+	logs := plog.NewLogs()
+	meta := p.Meta.KeyVal()
+	sl := logs.ResourceLogs().AppendEmpty().ScopeLogs().AppendEmpty()


Are we going to set some resource attributes that provide information about entities that generate telemetry, e.g. service.name, service.instance.id, etc, we can probably take it from Payload metadata. The current implementation will have the same resource metrics scope for all logs and traces produced by different services/processes/etc.

kovrus · 2023-03-03T11:02:00Z

receiver/faroreceiver/keyval.go

+}
+
+// KeyValAdd adds a key + value string pair to kv
+func KeyValAdd(kv *KeyVal, key string, value string) {


nit: can be keyVal method

kovrus · 2023-03-03T11:02:47Z

receiver/faroreceiver/keyval.go

+type KeyVal = om.OrderedMap
+
+// NewKeyVal creates new empty KeyVal
+func NewKeyVal() *KeyVal {


We should probably unexport specific to this component struct and functions or move it to the internal package.

runforesight · 2023-03-03T11:07:21Z

Foresight Summary

Major Impacts

build-and-test-windows duration(4 seconds) has decreased 31 minutes 40 seconds compared to main branch avg(31 minutes 44 seconds).

View More Details

✅ telemetrygen workflow has finished in 1 minute 1 second (1 minute 59 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

Job	Failed Steps	Tests
build-dev	- 🔗	N/A	See Details
publish-latest	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ check-links workflow has finished in 1 minute 25 seconds (57 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

Job	Failed Steps	Tests
changed files	- 🔗	N/A	See Details
check-links	- 🔗	N/A	See Details

❌ build-and-test workflow has finished in 44 minutes 8 seconds (26 minutes 25 seconds less than `main` branch avg.) and finished at 5th Mar, 2023. 3 jobs failed.

Job	Failed Steps	Tests
unittest-matrix (1.19, connector)	- 🔗	✅ 83 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, connector)	- 🔗	✅ 83 ❌ 0 ⏭ 0 🔗	See Details
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, internal)	- 🔗	✅ 581 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 538 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, processor)	- 🔗	✅ 1546 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, internal)	- 🔗	✅ 581 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, extension)	- 🔗	✅ 538 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, processor)	- 🔗	✅ 1546 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, receiver-0)	- 🔗	✅ 2580 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	✅ 2580 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, exporter)	- 🔗	✅ 2474 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, receiver-1)	- 🔗	✅ 1940 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, other)	- 🔗	✅ 4706 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	✅ 1940 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, exporter)	- 🔗	✅ 2474 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, other)	- 🔗	✅ 4706 ❌ 0 ⏭ 0 🔗	See Details
integration-tests	- 🔗	✅ 55 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
check-collector-module-version	Check Collector Module Version 🔗	N/A	See Details
check-codeowners	Check Code Owner Existence 🔗	N/A	See Details
checks	Porto 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
lint-matrix (receiver-0)	- 🔗	N/A	See Details
lint-matrix (receiver-1)	- 🔗	N/A	See Details
lint-matrix (processor)	- 🔗	N/A	See Details
lint-matrix (exporter)	- 🔗	N/A	See Details
lint-matrix (extension)	- 🔗	N/A	See Details
lint-matrix (connector)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	N/A	See Details
unittest (1.20)	- 🔗	N/A	See Details
unittest (1.19)	- 🔗	N/A	See Details
lint	- 🔗	N/A	See Details
cross-compile (darwin, amd64)	- 🔗	N/A	See Details
cross-compile (darwin, arm64)	- 🔗	N/A	See Details
cross-compile (linux, 386)	- 🔗	N/A	See Details
cross-compile (linux, amd64)	- 🔗	N/A	See Details
cross-compile (linux, arm)	- 🔗	N/A	See Details
cross-compile (linux, arm64)	- 🔗	N/A	See Details
cross-compile (linux, ppc64le)	- 🔗	N/A	See Details
cross-compile (windows, amd64)	- 🔗	N/A	See Details
cross-compile (windows, 386)	- 🔗	N/A	See Details
build-package (deb)	- 🔗	N/A	See Details
build-package (rpm)	- 🔗	N/A	See Details
windows-msi	- 🔗	N/A	See Details
publish-check	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details
publish-dev	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 13 minutes 35 seconds (⚠️ 4 minutes 55 seconds more than `main` branch avg.) and finished at 5th Mar, 2023.

Job	Failed Steps	Tests
prometheus-compliance-tests	- 🔗	✅ 21 ❌ 0 ⏭ 0 🔗	See Details

load-tests workflow has finished in 11 minutes 11 seconds (5 minutes 21 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

Job	Failed Steps	Tests
loadtest (TestIdleMode)	- 🔗	✅ 1 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceAttributesProcessor)	- 🔗	✅ 3 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetric10kDPS\|TestMetricsFromFile)	- 🔗	✅ 6 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceNoBackend10kSPS\|TestTrace1kSPSWithAttrs)	- 🔗	✅ 8 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetricResourceProcessor\|TestTrace10kSPS)	- 🔗	✅ 12 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceBallast1kSPSWithAttrs\|TestTraceBallast1kSPSAddAttrs)	- 🔗	✅ 10 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
loadtest (TestBallastMemory\|TestLog10kDPS)	- 🔗	✅ 18 ❌ 0 ⏭ 0 🔗	See Details

✅ e2e-tests workflow has finished in 19 minutes 5 seconds (⚠️ 2 minutes 47 seconds more than `main` branch avg.) and finished at 5th Mar, 2023.

Job	Failed Steps	Tests
kubernetes-test (v1.26.0)	- 🔗	N/A	See Details
kubernetes-test (v1.25.3)	- 🔗	N/A	See Details
kubernetes-test (v1.24.7)	- 🔗	N/A	See Details
kubernetes-test (v1.23.13)	- 🔗	N/A	See Details

⭕ build-and-test-windows workflow has finished in 4 seconds (31 minutes 40 seconds less than `main` branch avg.) and finished at 7th Apr, 2023.

Job	Failed Steps	Tests
windows-unittest-matrix	- 🔗	N/A	See Details
windows-unittest	- 🔗	N/A	See Details

❌ changelog workflow has finished in 44 seconds (1 minute 30 seconds less than `main` branch avg.) and finished at 7th Apr, 2023. 1 job failed.

Job	Failed Steps	Tests
changelog	Ensure ./.chloggen/*.yaml addition(s) 🔗	N/A	See Details

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

jpkrohling · 2023-03-03T11:59:42Z

receiver/faroreceiver/handler.go

+	"go.uber.org/zap"
+)
+
+type Handler struct {


Perhaps this can be moved to an internal package, or unexported?

jpkrohling · 2023-03-03T12:01:37Z

receiver/faroreceiver/handler.go

+
+	h.warnOnce.Do(func() {
+		if h.logConsumer == nil {
+			h.traceLogger.Warn("faroreceiver logs will not be consumed!")


The logger context has the receiver name already ("faro", "faro/2", ...), but it would be good to be clear why this is happening. Something like:

Suggested change

h.traceLogger.Warn("faroreceiver logs will not be consumed!")

h.traceLogger.Warn("this receiver wasn't added to a logs pipeline, logs will not be consumed")

jpkrohling · 2023-03-03T12:02:11Z

receiver/faroreceiver/handler.go

+			h.traceLogger.Warn("faroreceiver logs will not be consumed!")
+		}
+		if h.traceConsumer == nil {
+			h.logLogger.Warn("faroreceiver traces will not be consumed!")


Same here.

Suggested change

h.logLogger.Warn("faroreceiver traces will not be consumed!")

h.logLogger.Warn("this receiver wasn't added to a traces pipeline, traces will not be consumed")

jpkrohling · 2023-03-03T12:04:35Z

receiver/faroreceiver/handler.go

+	}
+
+	if err := h.consumePayload(ctx, p); err != nil {
+		http.Error(w, "collector error", http.StatusInternalServerError)


How about you also use err.Error() here, and be more specific on the error handling within the consumePayload func?

jpkrohling · 2023-03-03T12:05:35Z

receiver/faroreceiver/handler.go

+func (h *Handler) consumePayload(ctx context.Context, p Payload) error {
+	logs, err := translate(ctx, p)
+	if err != nil {
+		return err


Here, you could wrap (or just use a new error, if you are concerned about leaking details to the client), stating that:

return errors.New("failed to translate the payload into logs")

jpkrohling · 2023-03-03T12:07:25Z

receiver/faroreceiver/handler.go

+func commonAttributes() pcommon.Map {
+	attrs := pcommon.NewMap()
+	labels := []string{"kind"}
+	// TODO these shouldn't be hardcoded! this could be placed on logs by the attributeprocessor?


Indeed, the Faro receiver shouldn't be that closely tied to the Loki exporter.

jpkrohling · 2023-03-03T12:09:13Z

receiver/faroreceiver/payload.go

+	"go.opentelemetry.io/collector/pdata/ptrace"
+)
+
+type Kind string


Perhaps this could be part of a new pkg/translator? This way, other components (in potentially other repos) could reuse this logic?

Co-authored-by: Ruslan Kovalov <ruslan.kovalov@grafana.com>

atoulme · 2023-03-05T16:22:23Z

receiver/faroreceiver/factory.go

+	typeStr   = "faro"
+	stability = component.StabilityLevelDevelopment
+
+	defaultHTTPEndpoint = "0.0.0.0:8886"


Suggested change

defaultHTTPEndpoint = "0.0.0.0:8886"

defaultHTTPEndpoint = "localhost:8886"

atoulme · 2023-03-05T16:24:28Z

receiver/faroreceiver/factory.go

+
+func createDefaultConfig() component.Config {
+	return &Config{
+		HTTP: &confighttp.HTTPServerSettings{


Is there a reason to have the http server settings be nested under HTTP? Most just extend it.

atoulme · 2023-03-05T16:27:54Z

receiver/faroreceiver/faro.go

+
+	server       *http.Server
+	startOnce    sync.Once
+	shutdownOnce sync.Once


We don’t typically promise your receiver will start and stop once. There might be times where it is reloaded.

github-actions · 2023-05-26T22:03:09Z

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

github-actions · 2023-06-10T05:20:08Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[receiver/faro] add initial implementation of faroreceiver

fe4f397

Signed-off-by: Robbie Lankford <robert.lankford@grafana.com>

rlankfo requested review from a team and evan-bradley March 3, 2023 01:47

github-actions bot assigned Aneurysm9 Mar 3, 2023

add loggers to handler

a8bd212

kovrus reviewed Mar 3, 2023

View reviewed changes

jpkrohling reviewed Mar 3, 2023

View reviewed changes

Update receiver/faroreceiver/README.md

193acae

Co-authored-by: Ruslan Kovalov <ruslan.kovalov@grafana.com>

atoulme reviewed Mar 5, 2023

View reviewed changes

atoulme added the Accepted Component New component has been sponsored label Apr 7, 2023

github-actions bot added the Stale label May 26, 2023

github-actions bot closed this Jun 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[receiver/faro] add initial implementation of faroreceiver #19183

[receiver/faro] add initial implementation of faroreceiver #19183

rlankfo commented Mar 3, 2023

kovrus Mar 3, 2023 •

edited

Loading

kovrus Mar 3, 2023

kovrus Mar 3, 2023 •

edited

Loading

kovrus Mar 3, 2023

kovrus Mar 3, 2023

runforesight bot commented Mar 3, 2023 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 31 minutes 40 seconds compared to main branch avg(31 minutes 44 seconds).

✅ telemetrygen workflow has finished in 1 minute 1 second (1 minute 59 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

✅ check-links workflow has finished in 1 minute 25 seconds (57 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

❌ build-and-test workflow has finished in 44 minutes 8 seconds (26 minutes 25 seconds less than `main` branch avg.) and finished at 5th Mar, 2023. 3 jobs failed.

✅ prometheus-compliance-tests workflow has finished in 13 minutes 35 seconds (⚠️ 4 minutes 55 seconds more than `main` branch avg.) and finished at 5th Mar, 2023.

load-tests workflow has finished in 11 minutes 11 seconds (5 minutes 21 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

✅ e2e-tests workflow has finished in 19 minutes 5 seconds (⚠️ 2 minutes 47 seconds more than `main` branch avg.) and finished at 5th Mar, 2023.

⭕ build-and-test-windows workflow has finished in 4 seconds (31 minutes 40 seconds less than `main` branch avg.) and finished at 7th Apr, 2023.

❌ changelog workflow has finished in 44 seconds (1 minute 30 seconds less than `main` branch avg.) and finished at 7th Apr, 2023. 1 job failed.

jpkrohling Mar 3, 2023

jpkrohling Mar 3, 2023

jpkrohling Mar 3, 2023

jpkrohling Mar 3, 2023

jpkrohling Mar 3, 2023

jpkrohling Mar 3, 2023

jpkrohling Mar 3, 2023

atoulme Mar 5, 2023

atoulme Mar 5, 2023

atoulme Mar 5, 2023

github-actions bot commented May 26, 2023

github-actions bot commented Jun 10, 2023

	h.traceLogger.Warn("faroreceiver logs will not be consumed!")
	h.traceLogger.Warn("this receiver wasn't added to a logs pipeline, logs will not be consumed")

	h.logLogger.Warn("faroreceiver traces will not be consumed!")
	h.logLogger.Warn("this receiver wasn't added to a traces pipeline, traces will not be consumed")

	defaultHTTPEndpoint = "0.0.0.0:8886"
	defaultHTTPEndpoint = "localhost:8886"

[receiver/faro] add initial implementation of faroreceiver #19183

[receiver/faro] add initial implementation of faroreceiver #19183

Conversation

rlankfo commented Mar 3, 2023

kovrus Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kovrus Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runforesight bot commented Mar 3, 2023 • edited Loading

Foresight Summary

build-and-test-windows duration(4 seconds) has decreased 31 minutes 40 seconds compared to main branch avg(31 minutes 44 seconds).

✅ telemetrygen workflow has finished in 1 minute 1 second (1 minute 59 seconds less than main branch avg.) and finished at 5th Mar, 2023.

✅ check-links workflow has finished in 1 minute 25 seconds (57 seconds less than main branch avg.) and finished at 5th Mar, 2023.

❌ build-and-test workflow has finished in 44 minutes 8 seconds (26 minutes 25 seconds less than main branch avg.) and finished at 5th Mar, 2023. 3 jobs failed.

✅ prometheus-compliance-tests workflow has finished in 13 minutes 35 seconds (⚠️ 4 minutes 55 seconds more than main branch avg.) and finished at 5th Mar, 2023.

load-tests workflow has finished in 11 minutes 11 seconds (5 minutes 21 seconds less than main branch avg.) and finished at 5th Mar, 2023.

✅ e2e-tests workflow has finished in 19 minutes 5 seconds (⚠️ 2 minutes 47 seconds more than main branch avg.) and finished at 5th Mar, 2023.

⭕ build-and-test-windows workflow has finished in 4 seconds (31 minutes 40 seconds less than main branch avg.) and finished at 7th Apr, 2023.

❌ changelog workflow has finished in 44 seconds (1 minute 30 seconds less than main branch avg.) and finished at 7th Apr, 2023. 1 job failed.

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

github-actions bot commented May 26, 2023

github-actions bot commented Jun 10, 2023

kovrus Mar 3, 2023 •

edited

Loading

kovrus Mar 3, 2023 •

edited

Loading

runforesight bot commented Mar 3, 2023 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 31 minutes 40 seconds compared to main branch avg(31 minutes 44 seconds).

✅ telemetrygen workflow has finished in 1 minute 1 second (1 minute 59 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

✅ check-links workflow has finished in 1 minute 25 seconds (57 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

❌ build-and-test workflow has finished in 44 minutes 8 seconds (26 minutes 25 seconds less than `main` branch avg.) and finished at 5th Mar, 2023. 3 jobs failed.

✅ prometheus-compliance-tests workflow has finished in 13 minutes 35 seconds (⚠️ 4 minutes 55 seconds more than `main` branch avg.) and finished at 5th Mar, 2023.

load-tests workflow has finished in 11 minutes 11 seconds (5 minutes 21 seconds less than `main` branch avg.) and finished at 5th Mar, 2023.

✅ e2e-tests workflow has finished in 19 minutes 5 seconds (⚠️ 2 minutes 47 seconds more than `main` branch avg.) and finished at 5th Mar, 2023.

⭕ build-and-test-windows workflow has finished in 4 seconds (31 minutes 40 seconds less than `main` branch avg.) and finished at 7th Apr, 2023.

❌ changelog workflow has finished in 44 seconds (1 minute 30 seconds less than `main` branch avg.) and finished at 7th Apr, 2023. 1 job failed.