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

Simpler identity logging #4455

Merged
merged 5 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions sdk/identity/azure-identity/src/azure_cli_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,11 @@ AzureCliCredential::AzureCliCredential(

ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID");

auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + GetCredentialName()
+ " created.\n"
"Successful creation does not guarantee further successful token retrieval.");
}
Log::Write(
Logger::Level::Informational,
IdentityPrefix + GetCredentialName()
+ " created.\n"
"Successful creation does not guarantee further successful token retrieval.");
}

AzureCliCredential::AzureCliCredential(AzureCliCredentialOptions const& options)
Expand Down Expand Up @@ -168,12 +164,7 @@ AccessToken AzureCliCredential::GetToken(
auto const errorMsg
= IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"';

auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(logLevel, errorMsg);
}

Log::Write(Logger::Level::Warning, errorMsg);
throw AuthenticationException(errorMsg);
}
});
Expand Down
75 changes: 21 additions & 54 deletions sdk/identity/azure-identity/src/chained_token_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,67 +69,34 @@ AccessToken ChainedTokenCredentialImpl::GetToken(
TokenRequestContext const& tokenRequestContext,
Context const& context) const
{
auto const sourcesSize = m_sources.size();

if (sourcesSize == 0)
for (auto const& source : m_sources)
{
const auto logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
try
{
auto token = source->GetToken(tokenRequestContext, context);

Log::Write(
logLevel,
IdentityPrefix + credentialName
+ ": Authentication did not succeed: List of sources is empty.");
Logger::Level::Informational,
IdentityPrefix + credentialName + ": Successfully got token from "
+ source->GetCredentialName() + '.');

return token;
}
}
else
{
for (size_t i = 0; i < sourcesSize; ++i)
catch (AuthenticationException const& e)
{
try
{
auto token = m_sources[i]->GetToken(tokenRequestContext, context);

{
auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credentialName + ": Successfully got token from "
+ m_sources[i]->GetCredentialName() + '.');
}
}

return token;
}
catch (AuthenticationException const& e)
{
{
auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credentialName + ": Failed to get token from "
+ m_sources[i]->GetCredentialName() + ": " + e.what());
}
}

if ((sourcesSize - 1) == i) // On the last credential
{
auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credentialName
+ ": Didn't succeed to get a token from any credential in the chain.");
}
}
}
Log::Write(
Logger::Level::Verbose,
IdentityPrefix + credentialName + ": Failed to get token from "
+ source->GetCredentialName() + ": " + e.what());
}
}

Log::Write(
Logger::Level::Warning,
IdentityPrefix + credentialName
+ (m_sources.empty()
? ": Authentication did not succeed: List of sources is empty."
: ": Didn't succeed to get a token from any credential in the chain."));

throw AuthenticationException("Failed to get token from " + credentialName + '.');
}
29 changes: 13 additions & 16 deletions sdk/identity/azure-identity/src/default_azure_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,19 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt
{
// Initializing m_credential below and not in the member initializer list to have a specific order
// of log messages.
auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
std::string(IdentityPrefix) + "Creating " + GetCredentialName()
+ " which combines mutiple parameterless credentials into a single one.\n"
+ GetCredentialName()
+ " is only recommended for the early stages of development, "
"and not for usage in production environment."
"\nOnce the developer focuses on the Credentials and Authentication aspects "
"of their application, "
+ GetCredentialName()
+ " needs to be replaced with the credential that "
"is the better fit for the application.");
}

Log::Write(
Logger::Level::Verbose,
std::string(IdentityPrefix) + "Creating " + GetCredentialName()
+ " which combines mutiple parameterless credentials into a single one.\n"
+ GetCredentialName()
+ " is only recommended for the early stages of development, "
"and not for usage in production environment."
"\nOnce the developer focuses on the Credentials and Authentication aspects "
"of their application, "
+ GetCredentialName()
+ " needs to be replaced with the credential that "
"is the better fit for the application.");

// Creating credentials in order to ensure the order of log messages.
auto const envCred = std::make_shared<EnvironmentCredential>(options);
Expand Down
73 changes: 29 additions & 44 deletions sdk/identity/azure-identity/src/environment_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,38 +129,32 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options)

if (!m_credentialImpl)
{
auto const logLevel = Logger::Level::Warning;
Log::Write(
Logger::Level::Warning, logMsgPrefix + " was not initialized with underlying credential.");

auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
auto const basicMessage = logMsgPrefix + " was not initialized with underlying credential";

if (!Log::ShouldWrite(Logger::Level::Verbose))
{
Log::Write(logLevel, basicMessage + '.');
}
else
auto logMsg = logMsgPrefix + ": Both '" + AzureTenantIdEnvVarName + "' and '"
+ AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName
+ "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '"
+ AzureAuthorityHostEnvVarName
+ "' could be set to override the default authority host. Currently:\n";

std::pair<char const*, bool> envVarStatus[] = {
{AzureTenantIdEnvVarName, !tenantId.empty()},
{AzureClientIdEnvVarName, !clientId.empty()},
{AzureClientSecretEnvVarName, !clientSecret.empty()},
{AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()},
{AzureAuthorityHostEnvVarName, !authority.empty()},
};
for (auto const& status : envVarStatus)
{
auto logMsg = basicMessage + ": Both '" + AzureTenantIdEnvVarName + "' and '"
+ AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName
+ "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '"
+ AzureAuthorityHostEnvVarName
+ "' could be set to override the default authority host. Currently:\n";

std::pair<char const*, bool> envVarStatus[] = {
{AzureTenantIdEnvVarName, !tenantId.empty()},
{AzureClientIdEnvVarName, !clientId.empty()},
{AzureClientSecretEnvVarName, !clientSecret.empty()},
{AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()},
{AzureAuthorityHostEnvVarName, !authority.empty()},
};
for (auto const& status : envVarStatus)
{
logMsg += std::string(" * '") + status.first + "' " + "is"
+ (status.second ? " " : " NOT ") + "set\n";
}

Log::Write(logLevel, logMsg);
logMsg += std::string(" * '") + status.first + "' " + "is" + (status.second ? " " : " NOT ")
+ "set\n";
}

Log::Write(logLevel, logMsg);
}
}
}
Expand All @@ -174,15 +168,9 @@ AccessToken EnvironmentCredential::GetToken(
auto const AuthUnavailable
= IdentityPrefix + GetCredentialName() + " authentication unavailable. ";

{
auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details.");
}
}
Log::Write(
Logger::Level::Warning,
AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details.");

throw AuthenticationException(
AuthUnavailable + "Environment variables are not fully configured.");
Expand All @@ -197,15 +185,12 @@ void PrintCredentialCreationLogMessage(
std::vector<std::pair<char const*, char const*>> const& envVarsToParams,
char const* credThatGetsCreated)
{
Log::Write(
Logger::Level::Informational,
logMsgPrefix + " gets created with " + credThatGetsCreated + '.');

if (!Log::ShouldWrite(Logger::Level::Verbose))
{
if (Log::ShouldWrite(Logger::Level::Informational))
{
Log::Write(
Logger::Level::Informational,
logMsgPrefix + " gets created with " + credThatGetsCreated + '.');
}

return;
}

Expand Down
43 changes: 13 additions & 30 deletions sdk/identity/azure-identity/src/managed_identity_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,10 @@ std::string WithSourceMessage(std::string const& credSource)

void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& credSource)
{
auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credName + ": Environment is not set up for the credential to be created"
+ WithSourceMessage(credSource) + '.');
}
Log::Write(
Logger::Level::Verbose,
IdentityPrefix + credName + ": Environment is not set up for the credential to be created"
+ WithSourceMessage(credSource) + '.');
}
} // namespace

Expand All @@ -52,13 +48,9 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl(
{
auto const endpointUrl = Url(url);

auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.');
}
Log::Write(
Logger::Level::Informational,
IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.');

return endpointUrl;
}
Expand All @@ -73,12 +65,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl(
+ ": Failed to create: The environment variable \'" + envVarName
+ "\' contains an invalid URL.";

auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(logLevel, IdentityPrefix + errorMessage);
}

Log::Write(Logger::Level::Warning, IdentityPrefix + errorMessage);
throw AuthenticationException(errorMessage);
}

Expand Down Expand Up @@ -385,15 +372,11 @@ std::unique_ptr<ManagedIdentitySource> ImdsManagedIdentitySource::Create(
std::string const& clientId,
Azure::Core::Credentials::TokenCredentialOptions const& options)
{
auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credName + " will be created"
+ WithSourceMessage("Azure Instance Metadata Service")
+ ".\nSuccessful creation does not guarantee further successful token retrieval.");
}
Log::Write(
Logger::Level::Informational,
IdentityPrefix + credName + " will be created"
+ WithSourceMessage("Azure Instance Metadata Service")
+ ".\nSuccessful creation does not guarantee further successful token retrieval.");

return std::unique_ptr<ManagedIdentitySource>(new ImdsManagedIdentitySource(clientId, options));
}
Expand Down
Loading