Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1250 oauth2 tracing #1386

Merged
merged 9 commits into from
Feb 17, 2022
Prev Previous commit
Next Next commit
pass exporter index directly to exporter name generator
  • Loading branch information
Canuteson committed Feb 16, 2022
commit 66315cedacf53069d011c9176973a1506fbb1cfd
14 changes: 6 additions & 8 deletions pkg/traces/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,21 +372,21 @@ func exporter(rwCfg RemoteWriteConfig) (map[string]interface{}, error) {
return exporter, nil
}

func getExporterName(protocol string, format string) (string, error) {
func getExporterName(index int, protocol string, format string) (string, error) {
switch format {
case formatOtlp:
switch protocol {
case protocolGRPC:
return "otlp", nil
return fmt.Sprintf("otlp/%d", index), nil
case protocolHTTP:
return "otlphttp", nil
return fmt.Sprintf("otlphttp/%d", index), nil
default:
return "", errors.New("unknown protocol, expected either 'http' or 'grpc'")
}
case formatJaeger:
switch protocol {
case protocolGRPC:
return "jaeger", nil
return fmt.Sprintf("jaeger/%d", index), nil
default:
return "", errors.New("unknown protocol, expected 'grpc'")
}
Expand All @@ -403,11 +403,10 @@ func (c *InstanceConfig) exporters() (map[string]interface{}, error) {
if err != nil {
return nil, err
}
exporterName, err := getExporterName(remoteWriteConfig.Protocol, remoteWriteConfig.Format)
exporterName, err := getExporterName(i, remoteWriteConfig.Protocol, remoteWriteConfig.Format)
if err != nil {
return nil, err
}
exporterName = fmt.Sprintf("%s/%d", exporterName, i)
if remoteWriteConfig.Oauth2 != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes more sense to me in the exporter() function since we build the rest of the config there.

I believe it's here b/c the exporterName is here? Perhaps we could adjust exporter() to take its own name and use it?

Oof, it also seems odd that getExporterName() doesn't take i. Tacking it on afterwards seems error prone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I agree.
66315ce

exporter["auth"] = map[string]string{"authenticator": getAuthExtensionName(exporterName)}
}
Expand All @@ -427,15 +426,14 @@ func (c *InstanceConfig) extensions() (map[string]interface{}, error) {
if remoteWriteConfig.Oauth2 == nil {
continue
}
exporterName, err := getExporterName(remoteWriteConfig.Protocol, remoteWriteConfig.Format)
exporterName, err := getExporterName(i, remoteWriteConfig.Protocol, remoteWriteConfig.Format)
if err != nil {
return nil, err
}
oauthConfig, err := remoteWriteConfig.Oauth2.toOtelConfig()
if err != nil {
return nil, err
}
exporterName = fmt.Sprintf("%s/%d", exporterName, i)
extensions[getAuthExtensionName(exporterName)] = oauthConfig
}
return extensions, nil
Expand Down