grafana/phlare

When ingesting with duplicate labels, send error back to the client

eh-am opened this issue · 3 comments

eh-am commented

I was using the golang agent, passing a tag service_name, which collides with the ApplicationName (which is transformed into service_name).

	_, err := pyroscope.Start(pyroscope.Config{
		ApplicationName: appName,
		ServerAddress:   serverAddress,
		Logger:          pyroscope.StandardLogger,
		Tags: map[string]string{
			"service_name": "myservice",
		},
	})

The ingest server only returns 400, but no description. I think we should send back the error cause to the agent.

Here's the server error:

level=error caller=ingest_handler.go:57 ts=2023-05-31T11:23:40.162649Z msg="pyroscope ingest" err="pyroscopeIngesterAdapter failed to push: invalid_argument: profile with labels '{__delta__=\"false\", __name__=\"process_cpu\", foo=\"bar\", function=\"slow\", pyroscope_spy=\"gospy\", service_name=\"simple.golang.app\", service_name=\"myservice\"}' has duplicate label name: 'service_name'"

The server should allow this if service_name exists use it and keep appname as a label

eh-am commented

I personally feel it's fine to fail, I just want to have a better message to know what's wrong.

Isn't allowing service_name label kinda confusing?

  1. In a normal situation AppName becomes service_name
  2. If service_name label exists, then service_name is still service_name, but then AppName becomes app_name?

We were discussing changing the agents to deprecate AppName and use ServiceName instead, which in that case, what happens if both ServiceName AND service_name label are set?

I think appName as service_name is a fallback. But ultimately if someone has enforced service_name then that's what we should use, we can always keep other labels.