[receiver/azuremonitor] Add support for azureauthextension

[constanca-m]

Description

Add support for extension azureauth for authentication.

Link to tracking issue

Fixes #39048.

Testing

Unit tests added.

Documentation

README updated.

[celian-garcia]

Thank you for that effort! I will later probably remove the old one but that's ok to have both for a while.
I let you rule the red pipeline and answer my different comments.

[celian-garcia]

Thanks for that! So cool to see some tests of this quality for the config :)

[celian-garcia]

I think it can be clearer if you keep token_provider and auth separated bullet points. You can have a little duplication here no problem as I think I will remove auth later on.
Can you also put a deprecated flag on the auth one?

[constanca-m]

Yes, I have added it in the README, and also added a warning in the code:

opentelemetry-collector-contrib/receiver/azuremonitorreceiver/scraper.go

Line 189 in 356c20c

s.settings.Logger.Warn("'auth' is deprecated, use 'credentials' instead")

[celian-garcia]

I'm not sure that token_provider is the right term knowing that it does not generate token behind the scene. If you put that in others component though, I'm ok to keep same for consistency.

[constanca-m]

So far it is not use in any other components. Do you have a field name that you prefer?

[constanca-m]

I have changed to credentials. Is it better?

[celian-garcia]

I'm discovering a bit, but it looks like there is a recurring pattern.

auth:
  authenticator: <name of the extension>

There is a common structure for this in the collector https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configauth/configauth.go, it's used for all authentications.

That said this structure is dedicated to create http/grpc clients so what I would propose would be to create something like this:

• In the azure auth extension, copy this structure, with the same logic, and with a getter to obtain the creds.

// Authentication defines the auth settings for the azure receiver.
type Authentication struct {
	// AuthenticatorID specifies the name of the azure extension to use in order to authenticate the incoming data point.
	AuthenticatorID component.ID `mapstructure:"authenticator,omitempty"`
}

// GetAzureTokenCredential attempts to select the credential of the appropriate azure extension from the list of extensions,
// based on the requested extension name. If an authenticator is not found, an error is returned.
func (a Authentication) GetAzureTokenCredential(_ context.Context, extensions map[component.ID]component.Component) (*azcore.TokenCredential, error) {
	if ext, found := extensions[a.AuthenticatorID]; found {
		if cred, ok := ext.(azcore.TokenCredential); ok {
			return &cred, nil
		}
		return nil, errNotServer
	}

	return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound)
}

• In the azure monitor receiver, add a field in config with that type, with a azureauth name (to avoid breaking change and it tells well what it is)

azureauth:
  authenticator: azureauth/prd

WDYT?

[constanca-m]

The azureauth extension has the support for authenticator if the receiver requires client/server config (see this).

For this receiver, we actually need to use azcore.TokenCredential, which then implements this function.

So the easiest way would be to just replace the name credentials with authenticator in the receiver config (here). But I don't know if authenticator would be the best name, as we are not really using it as a client/server. Especially with authenticator being out of auth, which as you pointed out, is the common schema.

[constanca-m]

Renaming could work, thanks!

[celian-garcia]

I'm on it

[celian-garcia]

Here it is #39738

[celian-garcia]

They already merged it <3

[constanca-m]

Thanks @celian-garcia . I updated the PR now to use auth.authenticator.

[celian-garcia]

Great! just some nits and we should be good!
Thank you @constanca-m, I can't wait to remove all that crap 😈

[celian-garcia]

Can you keep just TenantID at this place? Just a nit but this will be clearer when we'll remove the deprecated part

[constanca-m]

Of course

[atoulme]

Please resolve conflicts and mark ready for review again.