From a58420b8bbd1e0f903310b964c1585b6980b29b5 Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Tue, 19 Nov 2024 14:13:37 +0000 Subject: [PATCH 01/31] Added MD file --- apps/design/logPraposal.md | 323 +++++++++++++++++++++++++++++++++++++ 1 file changed, 323 insertions(+) create mode 100644 apps/design/logPraposal.md diff --git a/apps/design/logPraposal.md b/apps/design/logPraposal.md new file mode 100644 index 00000000..0b76f815 --- /dev/null +++ b/apps/design/logPraposal.md @@ -0,0 +1,323 @@ +### Design Proposal: Managed Identity Authentication with Logging in Azure SDK + +#### **Overview** +This proposal outlines a design to implement two types of Managed Identity (MI) authentication mechanisms: one using the old Azure SDK and another leveraging the new SDK, with logging and structured logging (`log/slog`) integration. The focus is on demonstrating both methods, highlighting the improvements and how they can be applied in real-world scenarios. + +#### **Use Case** +We need to authenticate against Azure services using Managed Identity, particularly for `SystemAssigned` identity. The new SDK provides structured logging with the `log/slog` package, which helps with monitoring and debugging. We will implement both the old and new approaches, showing the differences and benefits of the new logging system. + +#### **Azure SDK Go** + +In the old Azure SDK approach, we utilize `ManagedIdentityCredential` to authenticate and acquire a token for Azure services. Here's an overview of how this works in practice: + +1. **Authentication**: + The `ManagedIdentityCredential` in the old SDK interacts with the IMDS (Instance Metadata Service) to authenticate the application and retrieve an access token for the Azure Resource Manager (ARM) API (`https://management.azure.com`). + +2. **Code Snippet**: + ```go + azlog.SetListener(func(event azlog.Event, s string){ + fmt.Println(": ", s) + }) + // Azure SDK Managed Identity Authentication + cred, err := azidentity.NewManagedIdentityCredential(nil) + if err != nil { + log.Fatalf("Failed to get ManagedIdentityCredential: %v", err) + } + + token, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{ + Scopes: []string{"https://management.azure.com/.default"}, + }) + if err != nil { + log.Fatalf("Failed to acquire token: %v", err) + } + fmt.Println("Token acquired:", token.Token) + ``` + + **Log Output**: + ```bash + : Managed Identity Credential will use IMDS managed identity + : NewDefaultAzureCredential failed to initialize some credentials: + EnvironmentCredential: missing environment variable AZURE_TENANT_ID + WorkloadIdentityCredential: no client ID specified. Check pod configuration or set ClientID in the options + : ManagedIdentityCredential.GetToken() acquired a token for scope "https://management.azure.com" + : DefaultAzureCredential authenticated with ManagedIdentityCredential + : ManagedIdentityCredential.GetToken() acquired a token for scope "https://management.azure.com" + ``` + +3. **Key Observations**: + - The old SDK uses a `NewDefaultAzureCredential` class for token acquisition. + - The logs are basic and do not provide structured logging, making it harder to troubleshoot and interpret. + +#### **New SDK Approach with Structured Logging** + +The new SDK introduces enhanced features like structured logging with `log/slog`. This approach offers better tracking and diagnostic capabilities. We’ll implement the Managed Identity using `mi.New()` with `SystemAssigned` identity and utilize structured logging. + +1. **Authentication with Structured Logging**: + The new SDK, with `log/slog`, integrates seamlessly with the token acquisition process. In this design, we will use `mi.New()` to initialize a `SystemAssigned` managed identity, while the log/slog captures the process and logs relevant information. + + +2. How to Here’s the complete code for your project that supports both callback-based logging for Go 1.18 and later and full `slog`-based logging for Go 1.21 and later. This version takes advantage of structured logging, multiple log levels, and flexible handling of logs with the `slog` package. + +--- + +### Project Structure +```plaintext +microsoft-authentication-library-for-go/apps/ +├── logger/ +│ ├── logger_118.go +│ ├── logger_121.go +│ ├── logger_wrapper.go +├── managedidentity/ +│ ├── managedidentity.go +├── main.go +``` + +--- + +### **1. `logger/logger_118.go`** (Callback Logging for Go 1.18 to 1.20) + +```go +//go:build go1.18 && !go1.21 + +package logger + +import "fmt" + +// CallbackFunc defines the signature for callback functions +// we can only have one string to support azure sdk +type CallbackFunc func(level, message string, fields ...any) + +// Logger struct for Go versions <= 1.20. +type Logger struct { + LogCallback CallbackFunc +} + +// Log method for Go <= 1.20, calls the callback function with log data. +func (a *Logger) Log(level string, message string, fields ...any) { + if a.LogCallback != nil { + a.LogCallback(level, message, fields...) + } else { + fmt.Println("No callback function provided") + } +} +``` + +--- + +### **2. `logger/logger_121.go`** (slog Logging for Go 1.21 and later) + +```go +//go:build go1.21 + +package logger + +import ( + "log/slog" +) + +// Logger struct for Go 1.21+ with full `slog` logging support. +type Logger struct { + loging *slog.Logger +} + +// Log method for Go 1.21+ with full support for structured logging and multiple log levels. +func (a *Logger) Log(level string, message string, fields ...any) { + if a.loging == nil { + return + } + + // Use the appropriate log level + var logEntry slog.Record + switch level { + case "info": + logEntry = slog.NewRecord(slog.LevelInfo, message, fields...) + a.loging.Log(logEntry) + case "error": + logEntry = slog.NewRecord(slog.LevelError, message, fields...) + a.loging.Log(logEntry) + case "warn": + logEntry = slog.NewRecord(slog.LevelWarn, message, fields...) + a.loging.Log(logEntry) + default: + logEntry = slog.NewRecord(slog.LevelInfo, "Default log level: "+message, fields...) + a.loging.Log(logEntry) + } +} +``` + +--- + +### **3. `logger/logger_wrapper.go`** (Factory Method for Creating Logger Instances) + +```go +package logger + +import ( + "fmt" + "log/slog" + "runtime" +) + +// New created a new logger instance, determining the Go version and choosing the appropriate logging method. +func New(input interface{}) (*Logger, error) { + if isGo121OrLater() { + if logger, ok := input.(*slog.Logger); ok { + return &Logger{Logger: logger}, nil + } + return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") + } + + if callback, ok := input.(func(level, message string, fields ...any)); ok { + return &Logger{LogCallback: callback}, nil + } + return nil, fmt.Errorf("invalid input for Go <=1.20; expected CallbackFunc") +} + +// isGo121OrLater checks if the Go version is 1.21 or later. +func isGo121OrLater() bool { + return runtime.Version() >= "go1.21" +} +``` + +--- +This is an example and can be use with any client. +### **4. `managedidentity/managedidentity.go`** (Configures the Logging Mechanism form `Logger`) + +```go +package managedidentity + +import ( + "fmt" + "microsoft-authentication-library-for-go/apps/logger" +) + +type Managedidentity struct { + logger *logger.Logger +} + +// Configure sets up the logger instance with either a callback or slog logger. +func (x *Client) New(input interface{}) error { + instance, err := logger.New(input) + if err != nil { + return fmt.Errorf("failed to configure logger: %w", err) + } + x.logger = instance + return nil +} + +// Log delegates the logging to the logger instance with specified level, message, and fields. +func (x *managedidentity) Log(level, message string, fields ...any) { + if x.logger == nil { + fmt.Println("logger instance is not configured") + return + } + x.logger.Log(level, message, fields...) +} +``` + +--- + +### **5. `main.go`** (Demonstrates Usage) + +```go +package main + +import ( + "fmt" + "log/slog" + "microsoft-authentication-library-for-go/apps/managedidentity" +) + +func main() { + miClient := &mi.Client{} + + // Configure Mi with a callback for Go <= 1.20 + callback := func(level, message string, fields ...any) { + // This callback simply prints the log level, message, and fields + fmt.Printf("[%s] %s ", level, message) + for _, field := range fields { + fmt.Printf("%v ", field) + } + fmt.Println() + } + if err := miClient.New(callback); err != nil { + fmt.Println("Error configuring Mi with callback:", err) + return + } + miClient.Log("info", "This is an info message via callback.", "username", "john_doe", "age", 30) + miClient.Log("error", "This is an error message via callback.", "module", "user-service") + + // Configure Mi with slog for Go 1.21+ + logger := slog.New(slog.NewTextHandler()) + if err := miClient.New(logger); err != nil { + fmt.Println("Error configuring Mi with slog:", err) + return + } + miClient.Log("info", "This is an info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) + miClient.Log("error", "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + miClient.Log("warn", "Disk space is low.", slog.Int("free_space_mb", 100)) + miClient.Log("info", "Default log message.", slog.String("module", "main")) +} +``` + +--- + +### **6. Expected Output** + +#### For **Go <= 1.20** (Callback-based Logging): + +```plaintext +[info] This is an info message via callback. username john_doe age 30 +[error] This is an error message via callback. module user-service +``` + +#### For **Go 1.21 and later** (slog-based Logging): + +```plaintext +INFO: This is an info message via slog. {"username": "john_doe", "age": 30} +ERROR: This is an error message via slog. {"module": "user-service", "retry": 3} +WARN: Disk space is low. {"free_space_mb": 100} +INFO: Default log message. {"module": "main"} +``` + +--- + +### Key Pointers: + +1. **Callback-based Logging for Go <= 1.20**: + - The `logger_118.go` file uses a callback function (`CallbackFunc`) to log messages. + - The callback prints the log level, message, and fields. + +2. **Full `slog` Support for Go 1.21+**: + - The `logger_121.go` file leverages the `slog` package, supporting structured logs, multiple log levels (`info`, `error`, `warn`), and fields. + +3. **Flexible Configuration**: + - The `managedidentity` module can be configured with either a callback (for Go <= 1.20) or a `slog.Logger` (for Go 1.21+). + +4. **Structured Logging**: + - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. + +This solution provides backward compatibility for Go 1.18 to 1.20 while fully leveraging the features of `slog` in Go 1.21 and beyond. + + +#### **Comparison of Old vs New Approaches** + +| Feature | Old SDK Approach | New SDK with Structured Logging | +|---------------------------------|-------------------------------------|---------------------------------| +| **Authentication Method** | `ManagedIdentityCredential` | `SystemAssigned` via `mi.New()` | +| **Logging** | Basic text logs | Structured logs using `log/slog` | +| **Log Output** | Simple log messages | Detailed, timestamped, structured logs | +| **Usability** | Standard approach | Enhanced debugging and monitoring | +| **Error Handling** | Simple error logs | Detailed error handling with contextual info | + +#### **Advantages of the New Approach** + +1. **Improved Diagnostics**: Structured logging provides detailed insights that are helpful for debugging and performance monitoring. +2. **Enhanced Monitoring**: With `log/slog`, logs can be better parsed and integrated into monitoring systems like Azure Monitor or ELK Stack. +3. **Ease of Use**: The new approach simplifies the interaction with Managed Identity credentials while adding more flexible logging. + +#### **Conclusion** + +The new SDK, with its use of structured logging through `log/slog`, greatly enhances the debugging and monitoring capabilities compared to the old SDK. This is especially useful in production environments where accurate, traceable logs are crucial for maintaining application health and troubleshooting issues. + +This design proposal suggests transitioning to the new SDK to leverage these benefits while maintaining compatibility with `SystemAssigned` Managed Identity authentication, also have support for the older call back functions that Azure sdk uses for 1.18 Go version. \ No newline at end of file From 57f484f00edd14467b28f8bf39fccf7addbcf9ca Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Thu, 21 Nov 2024 14:22:48 +0000 Subject: [PATCH 02/31] Updated callback support in 1.21 as well --- apps/design/logPraposal.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/apps/design/logPraposal.md b/apps/design/logPraposal.md index 0b76f815..200e307a 100644 --- a/apps/design/logPraposal.md +++ b/apps/design/logPraposal.md @@ -85,7 +85,7 @@ import "fmt" // CallbackFunc defines the signature for callback functions // we can only have one string to support azure sdk -type CallbackFunc func(level, message string, fields ...any) +type CallbackFunc func(level, message string) // Logger struct for Go versions <= 1.20. type Logger struct { @@ -111,6 +111,8 @@ func (a *Logger) Log(level string, message string, fields ...any) { package logger +type CallbackFunc func(level, message string) + import ( "log/slog" ) @@ -118,6 +120,7 @@ import ( // Logger struct for Go 1.21+ with full `slog` logging support. type Logger struct { loging *slog.Logger + callBackLogger CallbackFunc } // Log method for Go 1.21+ with full support for structured logging and multiple log levels. @@ -161,13 +164,16 @@ import ( // New created a new logger instance, determining the Go version and choosing the appropriate logging method. func New(input interface{}) (*Logger, error) { if isGo121OrLater() { + if callback, ok := input.(func(level, message string)); ok { + return &Logger{callBackLogger: callback}, nil + } if logger, ok := input.(*slog.Logger); ok { return &Logger{Logger: logger}, nil } return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") } - if callback, ok := input.(func(level, message string, fields ...any)); ok { + if callback, ok := input.(func(level, message string)); ok { return &Logger{LogCallback: callback}, nil } return nil, fmt.Errorf("invalid input for Go <=1.20; expected CallbackFunc") @@ -240,7 +246,7 @@ func main() { } fmt.Println() } - if err := miClient.New(callback); err != nil { + if err := miClient.New(SystemAssigned(), WithLogCallback(callback)); err != nil { fmt.Println("Error configuring Mi with callback:", err) return } @@ -249,7 +255,11 @@ func main() { // Configure Mi with slog for Go 1.21+ logger := slog.New(slog.NewTextHandler()) - if err := miClient.New(logger); err != nil { + if err := miClient.New(SystemAssigned(), WithLogger(logger); err != nil { + fmt.Println("Error configuring Mi with slog:", err) + return + } + if err := miClient.New(SystemAssigned(), WithLogCallback(callback); err != nil { fmt.Println("Error configuring Mi with slog:", err) return } From deb5d6e76eb2078177d63c5175f24e914f6286d6 Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Fri, 29 Nov 2024 11:20:50 +0000 Subject: [PATCH 03/31] Updated logging with files --- apps/logger/logger_118.go | 20 ++++++++++++++ apps/logger/logger_120.go | 50 +++++++++++++++++++++++++++++++++++ apps/logger/logger_wrapper.go | 36 +++++++++++++++++++++++++ 3 files changed, 106 insertions(+) create mode 100644 apps/logger/logger_118.go create mode 100644 apps/logger/logger_120.go create mode 100644 apps/logger/logger_wrapper.go diff --git a/apps/logger/logger_118.go b/apps/logger/logger_118.go new file mode 100644 index 00000000..d56f2fac --- /dev/null +++ b/apps/logger/logger_118.go @@ -0,0 +1,20 @@ +//go:build go1.18 && !go1.20 + +package logger + +import "fmt" + +// Logger struct for Go versions <= 1.20. +type Logger struct { + logCallback CallbackFunc +} + +// Log method for Go <= 1.20, calls the callback function with log. +func (a *Logger) Log(level string, message string, fields ...any) { + // We don't use fields in this version + if a.LogCallback != nil { + a.logCallback(level, message) + } else { + fmt.Println("No callback function provided") + } +} diff --git a/apps/logger/logger_120.go b/apps/logger/logger_120.go new file mode 100644 index 00000000..c4c8757e --- /dev/null +++ b/apps/logger/logger_120.go @@ -0,0 +1,50 @@ +//go:build go1.21 + +package logger + +import ( + "context" + "fmt" + "log/slog" +) + +// Logger struct for Go 1.21+ with full `slog` logging support. +type Logger struct { + logging *slog.Logger + logCallback CallbackFunc +} + +func New121(input interface{}) (*Logger, error) { + if logger, ok := input.(*slog.Logger); ok { + return &Logger{logging: logger}, nil + } + return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") +} + +// Log method for Go 1.21+ with full support for structured logging and multiple log levels. +func (a *Logger) Log(level Level, message string, fields ...any) { + if a.logging == nil { + return + } + var slogLevel slog.Level + switch level { + case Info: + slogLevel = slog.LevelInfo + case Err: + slogLevel = slog.LevelError + case Warn: + slogLevel = slog.LevelWarn + case Debug: + slogLevel = slog.LevelDebug + default: + slogLevel = slog.LevelInfo + } + + // Log the entry with the message and fields + a.logging.Log( + context.Background(), + slogLevel, + message, + fields..., + ) +} diff --git a/apps/logger/logger_wrapper.go b/apps/logger/logger_wrapper.go new file mode 100644 index 00000000..d5f967e5 --- /dev/null +++ b/apps/logger/logger_wrapper.go @@ -0,0 +1,36 @@ +package logger + +import ( + "fmt" + "runtime" +) + +// CallbackFunc defines the signature for callback functions +// we can only have one string to support azure sdk +type CallbackFunc func(level, message string) + +type Level string + +const ( + Info Level = "info" + Err Level = "err" + Warn Level = "warn" + Debug Level = "debug" +) + +// New created a new logger instance, determining the Go version and choosing the appropriate logging method. +func New(input interface{}) (*Logger, error) { + if isGo121OrLater() { + return New121(input) + } + + if callback, ok := input.(func(level, message string)); ok { + return &Logger{logCallback: callback}, nil + } + return nil, fmt.Errorf("invalid input for Go <=1.20; expected CallbackFunc") +} + +// isGo121OrLater checks if the Go version is 1.21 or later. +func isGo121OrLater() bool { + return runtime.Version() >= "go1.21" +} From e855af158fbc2ed023544a525260221a3e4c5ff0 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Thu, 19 Dec 2024 11:35:29 +0000 Subject: [PATCH 04/31] * Pushes changes needed for logging, including removing support for 1.18, and just focusing on the logger working on 1.21 and above * adds tests * refactors logger documentation * adds sample logger app --- apps/design/logPraposal.md | 333 --------------------- apps/design/logging.md | 36 +++ apps/logger/{logger_120.go => logger.go} | 29 +- apps/logger/logger_118.go | 20 -- apps/logger/logger_test.go | 56 ++++ apps/logger/logger_wrapper.go | 36 --- apps/tests/devapps/logger/logger_sample.go | 25 ++ 7 files changed, 138 insertions(+), 397 deletions(-) delete mode 100644 apps/design/logPraposal.md create mode 100644 apps/design/logging.md rename apps/logger/{logger_120.go => logger.go} (55%) delete mode 100644 apps/logger/logger_118.go create mode 100644 apps/logger/logger_test.go delete mode 100644 apps/logger/logger_wrapper.go create mode 100644 apps/tests/devapps/logger/logger_sample.go diff --git a/apps/design/logPraposal.md b/apps/design/logPraposal.md deleted file mode 100644 index 200e307a..00000000 --- a/apps/design/logPraposal.md +++ /dev/null @@ -1,333 +0,0 @@ -### Design Proposal: Managed Identity Authentication with Logging in Azure SDK - -#### **Overview** -This proposal outlines a design to implement two types of Managed Identity (MI) authentication mechanisms: one using the old Azure SDK and another leveraging the new SDK, with logging and structured logging (`log/slog`) integration. The focus is on demonstrating both methods, highlighting the improvements and how they can be applied in real-world scenarios. - -#### **Use Case** -We need to authenticate against Azure services using Managed Identity, particularly for `SystemAssigned` identity. The new SDK provides structured logging with the `log/slog` package, which helps with monitoring and debugging. We will implement both the old and new approaches, showing the differences and benefits of the new logging system. - -#### **Azure SDK Go** - -In the old Azure SDK approach, we utilize `ManagedIdentityCredential` to authenticate and acquire a token for Azure services. Here's an overview of how this works in practice: - -1. **Authentication**: - The `ManagedIdentityCredential` in the old SDK interacts with the IMDS (Instance Metadata Service) to authenticate the application and retrieve an access token for the Azure Resource Manager (ARM) API (`https://management.azure.com`). - -2. **Code Snippet**: - ```go - azlog.SetListener(func(event azlog.Event, s string){ - fmt.Println(": ", s) - }) - // Azure SDK Managed Identity Authentication - cred, err := azidentity.NewManagedIdentityCredential(nil) - if err != nil { - log.Fatalf("Failed to get ManagedIdentityCredential: %v", err) - } - - token, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{ - Scopes: []string{"https://management.azure.com/.default"}, - }) - if err != nil { - log.Fatalf("Failed to acquire token: %v", err) - } - fmt.Println("Token acquired:", token.Token) - ``` - - **Log Output**: - ```bash - : Managed Identity Credential will use IMDS managed identity - : NewDefaultAzureCredential failed to initialize some credentials: - EnvironmentCredential: missing environment variable AZURE_TENANT_ID - WorkloadIdentityCredential: no client ID specified. Check pod configuration or set ClientID in the options - : ManagedIdentityCredential.GetToken() acquired a token for scope "https://management.azure.com" - : DefaultAzureCredential authenticated with ManagedIdentityCredential - : ManagedIdentityCredential.GetToken() acquired a token for scope "https://management.azure.com" - ``` - -3. **Key Observations**: - - The old SDK uses a `NewDefaultAzureCredential` class for token acquisition. - - The logs are basic and do not provide structured logging, making it harder to troubleshoot and interpret. - -#### **New SDK Approach with Structured Logging** - -The new SDK introduces enhanced features like structured logging with `log/slog`. This approach offers better tracking and diagnostic capabilities. We’ll implement the Managed Identity using `mi.New()` with `SystemAssigned` identity and utilize structured logging. - -1. **Authentication with Structured Logging**: - The new SDK, with `log/slog`, integrates seamlessly with the token acquisition process. In this design, we will use `mi.New()` to initialize a `SystemAssigned` managed identity, while the log/slog captures the process and logs relevant information. - - -2. How to Here’s the complete code for your project that supports both callback-based logging for Go 1.18 and later and full `slog`-based logging for Go 1.21 and later. This version takes advantage of structured logging, multiple log levels, and flexible handling of logs with the `slog` package. - ---- - -### Project Structure -```plaintext -microsoft-authentication-library-for-go/apps/ -├── logger/ -│ ├── logger_118.go -│ ├── logger_121.go -│ ├── logger_wrapper.go -├── managedidentity/ -│ ├── managedidentity.go -├── main.go -``` - ---- - -### **1. `logger/logger_118.go`** (Callback Logging for Go 1.18 to 1.20) - -```go -//go:build go1.18 && !go1.21 - -package logger - -import "fmt" - -// CallbackFunc defines the signature for callback functions -// we can only have one string to support azure sdk -type CallbackFunc func(level, message string) - -// Logger struct for Go versions <= 1.20. -type Logger struct { - LogCallback CallbackFunc -} - -// Log method for Go <= 1.20, calls the callback function with log data. -func (a *Logger) Log(level string, message string, fields ...any) { - if a.LogCallback != nil { - a.LogCallback(level, message, fields...) - } else { - fmt.Println("No callback function provided") - } -} -``` - ---- - -### **2. `logger/logger_121.go`** (slog Logging for Go 1.21 and later) - -```go -//go:build go1.21 - -package logger - -type CallbackFunc func(level, message string) - -import ( - "log/slog" -) - -// Logger struct for Go 1.21+ with full `slog` logging support. -type Logger struct { - loging *slog.Logger - callBackLogger CallbackFunc -} - -// Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *Logger) Log(level string, message string, fields ...any) { - if a.loging == nil { - return - } - - // Use the appropriate log level - var logEntry slog.Record - switch level { - case "info": - logEntry = slog.NewRecord(slog.LevelInfo, message, fields...) - a.loging.Log(logEntry) - case "error": - logEntry = slog.NewRecord(slog.LevelError, message, fields...) - a.loging.Log(logEntry) - case "warn": - logEntry = slog.NewRecord(slog.LevelWarn, message, fields...) - a.loging.Log(logEntry) - default: - logEntry = slog.NewRecord(slog.LevelInfo, "Default log level: "+message, fields...) - a.loging.Log(logEntry) - } -} -``` - ---- - -### **3. `logger/logger_wrapper.go`** (Factory Method for Creating Logger Instances) - -```go -package logger - -import ( - "fmt" - "log/slog" - "runtime" -) - -// New created a new logger instance, determining the Go version and choosing the appropriate logging method. -func New(input interface{}) (*Logger, error) { - if isGo121OrLater() { - if callback, ok := input.(func(level, message string)); ok { - return &Logger{callBackLogger: callback}, nil - } - if logger, ok := input.(*slog.Logger); ok { - return &Logger{Logger: logger}, nil - } - return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") - } - - if callback, ok := input.(func(level, message string)); ok { - return &Logger{LogCallback: callback}, nil - } - return nil, fmt.Errorf("invalid input for Go <=1.20; expected CallbackFunc") -} - -// isGo121OrLater checks if the Go version is 1.21 or later. -func isGo121OrLater() bool { - return runtime.Version() >= "go1.21" -} -``` - ---- -This is an example and can be use with any client. -### **4. `managedidentity/managedidentity.go`** (Configures the Logging Mechanism form `Logger`) - -```go -package managedidentity - -import ( - "fmt" - "microsoft-authentication-library-for-go/apps/logger" -) - -type Managedidentity struct { - logger *logger.Logger -} - -// Configure sets up the logger instance with either a callback or slog logger. -func (x *Client) New(input interface{}) error { - instance, err := logger.New(input) - if err != nil { - return fmt.Errorf("failed to configure logger: %w", err) - } - x.logger = instance - return nil -} - -// Log delegates the logging to the logger instance with specified level, message, and fields. -func (x *managedidentity) Log(level, message string, fields ...any) { - if x.logger == nil { - fmt.Println("logger instance is not configured") - return - } - x.logger.Log(level, message, fields...) -} -``` - ---- - -### **5. `main.go`** (Demonstrates Usage) - -```go -package main - -import ( - "fmt" - "log/slog" - "microsoft-authentication-library-for-go/apps/managedidentity" -) - -func main() { - miClient := &mi.Client{} - - // Configure Mi with a callback for Go <= 1.20 - callback := func(level, message string, fields ...any) { - // This callback simply prints the log level, message, and fields - fmt.Printf("[%s] %s ", level, message) - for _, field := range fields { - fmt.Printf("%v ", field) - } - fmt.Println() - } - if err := miClient.New(SystemAssigned(), WithLogCallback(callback)); err != nil { - fmt.Println("Error configuring Mi with callback:", err) - return - } - miClient.Log("info", "This is an info message via callback.", "username", "john_doe", "age", 30) - miClient.Log("error", "This is an error message via callback.", "module", "user-service") - - // Configure Mi with slog for Go 1.21+ - logger := slog.New(slog.NewTextHandler()) - if err := miClient.New(SystemAssigned(), WithLogger(logger); err != nil { - fmt.Println("Error configuring Mi with slog:", err) - return - } - if err := miClient.New(SystemAssigned(), WithLogCallback(callback); err != nil { - fmt.Println("Error configuring Mi with slog:", err) - return - } - miClient.Log("info", "This is an info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) - miClient.Log("error", "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - miClient.Log("warn", "Disk space is low.", slog.Int("free_space_mb", 100)) - miClient.Log("info", "Default log message.", slog.String("module", "main")) -} -``` - ---- - -### **6. Expected Output** - -#### For **Go <= 1.20** (Callback-based Logging): - -```plaintext -[info] This is an info message via callback. username john_doe age 30 -[error] This is an error message via callback. module user-service -``` - -#### For **Go 1.21 and later** (slog-based Logging): - -```plaintext -INFO: This is an info message via slog. {"username": "john_doe", "age": 30} -ERROR: This is an error message via slog. {"module": "user-service", "retry": 3} -WARN: Disk space is low. {"free_space_mb": 100} -INFO: Default log message. {"module": "main"} -``` - ---- - -### Key Pointers: - -1. **Callback-based Logging for Go <= 1.20**: - - The `logger_118.go` file uses a callback function (`CallbackFunc`) to log messages. - - The callback prints the log level, message, and fields. - -2. **Full `slog` Support for Go 1.21+**: - - The `logger_121.go` file leverages the `slog` package, supporting structured logs, multiple log levels (`info`, `error`, `warn`), and fields. - -3. **Flexible Configuration**: - - The `managedidentity` module can be configured with either a callback (for Go <= 1.20) or a `slog.Logger` (for Go 1.21+). - -4. **Structured Logging**: - - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. - -This solution provides backward compatibility for Go 1.18 to 1.20 while fully leveraging the features of `slog` in Go 1.21 and beyond. - - -#### **Comparison of Old vs New Approaches** - -| Feature | Old SDK Approach | New SDK with Structured Logging | -|---------------------------------|-------------------------------------|---------------------------------| -| **Authentication Method** | `ManagedIdentityCredential` | `SystemAssigned` via `mi.New()` | -| **Logging** | Basic text logs | Structured logs using `log/slog` | -| **Log Output** | Simple log messages | Detailed, timestamped, structured logs | -| **Usability** | Standard approach | Enhanced debugging and monitoring | -| **Error Handling** | Simple error logs | Detailed error handling with contextual info | - -#### **Advantages of the New Approach** - -1. **Improved Diagnostics**: Structured logging provides detailed insights that are helpful for debugging and performance monitoring. -2. **Enhanced Monitoring**: With `log/slog`, logs can be better parsed and integrated into monitoring systems like Azure Monitor or ELK Stack. -3. **Ease of Use**: The new approach simplifies the interaction with Managed Identity credentials while adding more flexible logging. - -#### **Conclusion** - -The new SDK, with its use of structured logging through `log/slog`, greatly enhances the debugging and monitoring capabilities compared to the old SDK. This is especially useful in production environments where accurate, traceable logs are crucial for maintaining application health and troubleshooting issues. - -This design proposal suggests transitioning to the new SDK to leverage these benefits while maintaining compatibility with `SystemAssigned` Managed Identity authentication, also have support for the older call back functions that Azure sdk uses for 1.18 Go version. \ No newline at end of file diff --git a/apps/design/logging.md b/apps/design/logging.md new file mode 100644 index 00000000..e2bc6a94 --- /dev/null +++ b/apps/design/logging.md @@ -0,0 +1,36 @@ +# Logging + +GO 1.21 introduces enhanced features like structured logging with `log/slog`. +As part of MSAL GO, we have decided to only support logging using slog from version 1.21. +The reason for this is `log/slog` greatly enhances the debugging and monitoring capabilities compared to the old SDK. This is especially useful in production environments where accurate, traceable logs are crucial for maintaining application health and troubleshooting issues. For versions older than 1.21, the user can specify *nil* to the *New()* function and a no-op is returned + +## **Expected Output** + +```plaintext +time=2024-12-19T01:25:58.730Z level=INFO msg="This is an info message via slog." username=john_doe age=30 +time=2024-12-19T01:25:58.730Z level=ERROR msg="This is an error message via slog." module=user-service retry=3 +time=2024-12-19T01:25:58.730Z level=WARN msg="Disk space is low." free_space_mb=100 +time=2024-12-19T01:25:58.730Z level=INFO msg="Default log message." module=main +``` + +## Key Pointers + +1. **For Go <= 1.20**: + - User should pass *nil* to `logger.New(nil)` + - No-op is returned and can be handled by user + +2. **Full `slog` Support for Go 1.21+**: + - The `logger.go` file leverages the `slog` package, supporting structured logs, multiple log levels (`info`, `error`, `warn`), and fields. + +3. **Structured Logging**: + - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. + +### **Sample** + +A sample of the logger can be found in the following location: + +```plaintext +microsoft-authentication-library-for-go/apps/devapps +├── logger/ +│ ├── logger_sample.go +``` diff --git a/apps/logger/logger_120.go b/apps/logger/logger.go similarity index 55% rename from apps/logger/logger_120.go rename to apps/logger/logger.go index c4c8757e..c8bda55f 100644 --- a/apps/logger/logger_120.go +++ b/apps/logger/logger.go @@ -1,5 +1,3 @@ -//go:build go1.21 - package logger import ( @@ -8,17 +6,32 @@ import ( "log/slog" ) +// CallbackFunc defines the signature for callback functions +// we can only have one string to support azure sdk +type CallbackFunc func(level, message string) + +type Level string + +const ( + Info Level = "info" + Err Level = "error" + Warn Level = "warn" + Debug Level = "debug" +) + // Logger struct for Go 1.21+ with full `slog` logging support. type Logger struct { - logging *slog.Logger - logCallback CallbackFunc + logging *slog.Logger } -func New121(input interface{}) (*Logger, error) { - if logger, ok := input.(*slog.Logger); ok { - return &Logger{logging: logger}, nil +// New creates a new logger instance +func New(slogLogger *slog.Logger) (*Logger, error) { + // Return a logger instance for Go 1.21+ + if slogLogger == nil { + return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") } - return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") + + return &Logger{logging: slogLogger}, nil } // Log method for Go 1.21+ with full support for structured logging and multiple log levels. diff --git a/apps/logger/logger_118.go b/apps/logger/logger_118.go deleted file mode 100644 index d56f2fac..00000000 --- a/apps/logger/logger_118.go +++ /dev/null @@ -1,20 +0,0 @@ -//go:build go1.18 && !go1.20 - -package logger - -import "fmt" - -// Logger struct for Go versions <= 1.20. -type Logger struct { - logCallback CallbackFunc -} - -// Log method for Go <= 1.20, calls the callback function with log. -func (a *Logger) Log(level string, message string, fields ...any) { - // We don't use fields in this version - if a.LogCallback != nil { - a.logCallback(level, message) - } else { - fmt.Println("No callback function provided") - } -} diff --git a/apps/logger/logger_test.go b/apps/logger/logger_test.go new file mode 100644 index 00000000..439ddcd9 --- /dev/null +++ b/apps/logger/logger_test.go @@ -0,0 +1,56 @@ +package logger + +import ( + "bytes" + "log/slog" + "testing" +) + +func TestLogger_Log_ConsoleOutput(t *testing.T) { + // Capture the console output + var buf bytes.Buffer + handlerOptions := &slog.HandlerOptions{ + Level: slog.LevelDebug, // Set the log level to Debug to capture all log levels + } + slogLogger := slog.New(slog.NewTextHandler(&buf, handlerOptions)) + + // Create a new logger instance + logInstance, err := New(slogLogger) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + // Log messages + logInstance.Log(Info, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) + logInstance.Log(Err, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + logInstance.Log(Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + logInstance.Log(Debug, "This is a debug message via slog.", slog.String("module", "main")) + + // Check the output + output := buf.String() + expectedMessages := []string{ + "This is a info message via slog.", + "This is a error message via slog.", + "This is a warn message via slog.", + "This is a debug message via slog.", + } + + for _, msg := range expectedMessages { + if !bytes.Contains([]byte(output), []byte(msg)) { + t.Errorf("expected log message %q not found in output", msg) + } + } +} + +// This test is to emulate what happens if the user has a go version < 1.21 +// In this case, they will not have access to slog and will need to pass nil to the New function +func TestLogger_New_NilLogger(t *testing.T) { + // Attempt to create a new logger instance with nil slog.Logger + logInstance, err := New(nil) + if err == nil { + t.Fatalf("expected error, got nil") + } + if logInstance != nil { + t.Fatalf("expected nil logInstance, got %v", logInstance) + } +} diff --git a/apps/logger/logger_wrapper.go b/apps/logger/logger_wrapper.go deleted file mode 100644 index d5f967e5..00000000 --- a/apps/logger/logger_wrapper.go +++ /dev/null @@ -1,36 +0,0 @@ -package logger - -import ( - "fmt" - "runtime" -) - -// CallbackFunc defines the signature for callback functions -// we can only have one string to support azure sdk -type CallbackFunc func(level, message string) - -type Level string - -const ( - Info Level = "info" - Err Level = "err" - Warn Level = "warn" - Debug Level = "debug" -) - -// New created a new logger instance, determining the Go version and choosing the appropriate logging method. -func New(input interface{}) (*Logger, error) { - if isGo121OrLater() { - return New121(input) - } - - if callback, ok := input.(func(level, message string)); ok { - return &Logger{logCallback: callback}, nil - } - return nil, fmt.Errorf("invalid input for Go <=1.20; expected CallbackFunc") -} - -// isGo121OrLater checks if the Go version is 1.21 or later. -func isGo121OrLater() bool { - return runtime.Version() >= "go1.21" -} diff --git a/apps/tests/devapps/logger/logger_sample.go b/apps/tests/devapps/logger/logger_sample.go new file mode 100644 index 00000000..972484fd --- /dev/null +++ b/apps/tests/devapps/logger/logger_sample.go @@ -0,0 +1,25 @@ +package main + +import ( + "fmt" + "log/slog" + "os" + + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/logger" +) + +func main() { + // Test for Go 1.21+ + handlerOptions := &slog.HandlerOptions{} + slogLogger := slog.New(slog.NewTextHandler(os.Stdout, handlerOptions)) + logInstance, err := logger.New(slogLogger) + if err != nil { + fmt.Println("Error creating logger with slog:", err) + return + } + + logInstance.Log(logger.Info, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) + logInstance.Log(logger.Err, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + logInstance.Log(logger.Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + logInstance.Log(logger.Debug, "This is a debug message via slog.", slog.String("module", "main")) +} From 0ba0540f08bd1fa2565d4572e1363e296bdfc6bb Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 23 Dec 2024 15:27:27 +0000 Subject: [PATCH 05/31] * Changes how logger works so that we can create a logger implementation for 1.21+ * Update tests --- apps/confidential/confidential.go | 20 +++++++ apps/confidential/confidential_test.go | 3 ++ apps/internal/logger/logger.go | 21 ++++++++ apps/internal/logger/logger_121_below.go | 21 ++++++++ apps/internal/logger/logger_121_plus.go | 60 +++++++++++++++++++++ apps/{ => internal}/logger/logger_test.go | 27 +++++----- apps/logger/logger.go | 63 ---------------------- apps/tests/devapps/logger/logger_sample.go | 2 +- 8 files changed, 140 insertions(+), 77 deletions(-) create mode 100644 apps/internal/logger/logger.go create mode 100644 apps/internal/logger/logger_121_below.go create mode 100644 apps/internal/logger/logger_121_plus.go rename apps/{ => internal}/logger/logger_test.go (55%) delete mode 100644 apps/logger/logger.go diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 57d0e277..a2489ac9 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -24,6 +24,7 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/base" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/exported" + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/accesstokens" @@ -250,11 +251,24 @@ type clientOptions struct { capabilities []string disableInstanceDiscovery, sendX5C bool httpClient ops.HTTPClient + logger logger.LoggerInterface } // Option is an optional argument to New(). type Option func(o *clientOptions) +//  WithLogger allows for a custom logger to be set. +func WithLogger(l interface{}) Option { + return func(o *clientOptions) { + logInstance, err := logger.New(l) + if err != nil { + fmt.Println("Error creating logger with slog:", err) + return + } + o.logger = logInstance + } +} + // WithCache provides an accessor that will read and write authentication data to an externally managed cache. func WithCache(accessor cache.ExportReplace) Option { return func(o *clientOptions) { @@ -318,12 +332,17 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client return Client{}, err } autoEnabledRegion := os.Getenv("MSAL_FORCE_REGION") + defaultLogger, err := logger.New(nil) + if err != nil { + return Client{}, err + } opts := clientOptions{ authority: authority, // if the caller specified a token provider, it will handle all details of authentication, using Client only as a token cache disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, azureRegion: autoEnabledRegion, + logger: defaultLogger, } for _, o := range options { o(&opts) @@ -345,6 +364,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true + opts.logger.Log(context.Background(), logger.Info, "Created confidential client", logger.Field("authority", opts.authority), logger.Field("clientID", clientID)) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index 9a9c9ee1..bc1c5d77 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -182,6 +182,9 @@ func TestRegionAutoEnable_EmptyRegion_EnvRegion(t *testing.T) { } defer os.Unsetenv("MSAL_FORCE_REGION") + // handlerOptions := &slog.HandlerOptions{} + // slogLogger := slog.New(slog.NewTextHandler(os.Stdout, handlerOptions)) + lmo := "login.microsoftonline.com" tenant := "tenant" mockClient := mock.Client{} diff --git a/apps/internal/logger/logger.go b/apps/internal/logger/logger.go new file mode 100644 index 00000000..59c488a4 --- /dev/null +++ b/apps/internal/logger/logger.go @@ -0,0 +1,21 @@ +package logger + +import "context" + +type Level string + +const ( + Info Level = "info" + Err Level = "error" + Warn Level = "warn" + Debug Level = "debug" +) + +// LoggerInterface defines the methods that a logger should implement +type LoggerInterface interface { + Log(ctx context.Context, level Level, message string, fields ...any) +} + +func New(loggerInterface interface{}) (LoggerInterface, error) { + return NewLogger(loggerInterface) +} diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/logger/logger_121_below.go new file mode 100644 index 00000000..f66e811a --- /dev/null +++ b/apps/internal/logger/logger_121_below.go @@ -0,0 +1,21 @@ +//go:build go1.18 && !go1.21 + +package logger + +import "context" + +type logger struct{} + +func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { + return &logger{}, nil +} + +// Log method for Go 1.21+ with full support for structured logging and multiple log levels. +func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { + return +} + +// Field creates a slog field for any value +func Field(key string, value any) any { + return "" +} diff --git a/apps/internal/logger/logger_121_plus.go b/apps/internal/logger/logger_121_plus.go new file mode 100644 index 00000000..e807cea3 --- /dev/null +++ b/apps/internal/logger/logger_121_plus.go @@ -0,0 +1,60 @@ +//go:build go1.21 + +package logger + +import ( + "context" + "fmt" + "log/slog" +) + +// logger struct for Go 1.21+ with full `slog` logging support. +type logger struct { + logging *slog.Logger +} + +// New creates a new logger instance +func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { + if loggerInterface == nil { + return &logger{logging: nil}, nil + } + + if loggerInterface, ok := loggerInterface.(*slog.Logger); ok { + return &logger{logging: loggerInterface}, nil + } + + return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") +} + +// Log method for Go 1.21+ with full support for structured logging and multiple log levels. +func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { + if a == nil || a.logging == nil { + return + } + var slogLevel slog.Level + switch level { + case Info: + slogLevel = slog.LevelInfo + case Err: + slogLevel = slog.LevelError + case Warn: + slogLevel = slog.LevelWarn + case Debug: + slogLevel = slog.LevelDebug + default: + slogLevel = slog.LevelInfo + } + + // Log the entry with the message and fields + a.logging.Log( + ctx, + slogLevel, + message, + fields..., + ) +} + +// Field creates a slog field for any value +func Field(key string, value any) any { + return slog.Any(key, value) +} diff --git a/apps/logger/logger_test.go b/apps/internal/logger/logger_test.go similarity index 55% rename from apps/logger/logger_test.go rename to apps/internal/logger/logger_test.go index 439ddcd9..508f1583 100644 --- a/apps/logger/logger_test.go +++ b/apps/internal/logger/logger_test.go @@ -2,6 +2,7 @@ package logger import ( "bytes" + "context" "log/slog" "testing" ) @@ -9,28 +10,28 @@ import ( func TestLogger_Log_ConsoleOutput(t *testing.T) { // Capture the console output var buf bytes.Buffer - handlerOptions := &slog.HandlerOptions{ + handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{ Level: slog.LevelDebug, // Set the log level to Debug to capture all log levels - } - slogLogger := slog.New(slog.NewTextHandler(&buf, handlerOptions)) + }) // Create a new logger instance + slogLogger := slog.New(handler) logInstance, err := New(slogLogger) if err != nil { t.Fatalf("expected no error, got %v", err) } // Log messages - logInstance.Log(Info, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) - logInstance.Log(Err, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - logInstance.Log(Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - logInstance.Log(Debug, "This is a debug message via slog.", slog.String("module", "main")) + logInstance.Log(context.Background(), Info, "This is an info message via slog.", Field("username", "john_doe"), slog.Int("age", 30)) + logInstance.Log(context.Background(), Err, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + logInstance.Log(context.Background(), Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + logInstance.Log(context.Background(), Debug, "This is a debug message via slog.", slog.String("module", "main")) // Check the output output := buf.String() expectedMessages := []string{ - "This is a info message via slog.", - "This is a error message via slog.", + "This is an info message via slog.", + "This is an error message via slog.", "This is a warn message via slog.", "This is a debug message via slog.", } @@ -47,10 +48,10 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { func TestLogger_New_NilLogger(t *testing.T) { // Attempt to create a new logger instance with nil slog.Logger logInstance, err := New(nil) - if err == nil { - t.Fatalf("expected error, got nil") + if err != nil { + t.Fatalf("expected no error, got %v", err) } - if logInstance != nil { - t.Fatalf("expected nil logInstance, got %v", logInstance) + if logInstance == nil { + t.Fatalf("expected non-nil logInstance, got nil") } } diff --git a/apps/logger/logger.go b/apps/logger/logger.go deleted file mode 100644 index c8bda55f..00000000 --- a/apps/logger/logger.go +++ /dev/null @@ -1,63 +0,0 @@ -package logger - -import ( - "context" - "fmt" - "log/slog" -) - -// CallbackFunc defines the signature for callback functions -// we can only have one string to support azure sdk -type CallbackFunc func(level, message string) - -type Level string - -const ( - Info Level = "info" - Err Level = "error" - Warn Level = "warn" - Debug Level = "debug" -) - -// Logger struct for Go 1.21+ with full `slog` logging support. -type Logger struct { - logging *slog.Logger -} - -// New creates a new logger instance -func New(slogLogger *slog.Logger) (*Logger, error) { - // Return a logger instance for Go 1.21+ - if slogLogger == nil { - return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") - } - - return &Logger{logging: slogLogger}, nil -} - -// Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *Logger) Log(level Level, message string, fields ...any) { - if a.logging == nil { - return - } - var slogLevel slog.Level - switch level { - case Info: - slogLevel = slog.LevelInfo - case Err: - slogLevel = slog.LevelError - case Warn: - slogLevel = slog.LevelWarn - case Debug: - slogLevel = slog.LevelDebug - default: - slogLevel = slog.LevelInfo - } - - // Log the entry with the message and fields - a.logging.Log( - context.Background(), - slogLevel, - message, - fields..., - ) -} diff --git a/apps/tests/devapps/logger/logger_sample.go b/apps/tests/devapps/logger/logger_sample.go index 972484fd..9c536afd 100644 --- a/apps/tests/devapps/logger/logger_sample.go +++ b/apps/tests/devapps/logger/logger_sample.go @@ -5,7 +5,7 @@ import ( "log/slog" "os" - "github.com/AzureAD/microsoft-authentication-library-for-go/apps/logger" + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" ) func main() { From 77d735e6e05e6dbf57a0b4ec43e1b7ec7f48151d Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 23 Dec 2024 15:33:41 +0000 Subject: [PATCH 06/31] * remove unneeded log --- apps/confidential/confidential.go | 2 +- apps/confidential/confidential_test.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index a2489ac9..3328a6df 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -364,7 +364,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true - opts.logger.Log(context.Background(), logger.Info, "Created confidential client", logger.Field("authority", opts.authority), logger.Field("clientID", clientID)) + opts.logger.Log(context.Background(), logger.Info, "Created confidential client", logger.Field("clientID", clientID)) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index bc1c5d77..9a9c9ee1 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -182,9 +182,6 @@ func TestRegionAutoEnable_EmptyRegion_EnvRegion(t *testing.T) { } defer os.Unsetenv("MSAL_FORCE_REGION") - // handlerOptions := &slog.HandlerOptions{} - // slogLogger := slog.New(slog.NewTextHandler(os.Stdout, handlerOptions)) - lmo := "login.microsoftonline.com" tenant := "tenant" mockClient := mock.Client{} From e6aa078486de54630d9f08842dc644f0d26a8ac0 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 23 Dec 2024 16:08:03 +0000 Subject: [PATCH 07/31] * moves context.Background to internal implementation --- apps/confidential/confidential.go | 2 +- apps/internal/logger/logger.go | 4 +--- apps/internal/logger/logger_121_below.go | 4 +--- apps/internal/logger/logger_121_plus.go | 4 ++-- apps/internal/logger/logger_test.go | 9 ++++----- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 3328a6df..d21edd8b 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -364,7 +364,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true - opts.logger.Log(context.Background(), logger.Info, "Created confidential client", logger.Field("clientID", clientID)) + opts.logger.Log(logger.Info, "Created confidential client", logger.Field("clientID", clientID)) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/internal/logger/logger.go b/apps/internal/logger/logger.go index 59c488a4..cd8dbdee 100644 --- a/apps/internal/logger/logger.go +++ b/apps/internal/logger/logger.go @@ -1,7 +1,5 @@ package logger -import "context" - type Level string const ( @@ -13,7 +11,7 @@ const ( // LoggerInterface defines the methods that a logger should implement type LoggerInterface interface { - Log(ctx context.Context, level Level, message string, fields ...any) + Log(level Level, message string, fields ...any) } func New(loggerInterface interface{}) (LoggerInterface, error) { diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/logger/logger_121_below.go index f66e811a..a1bfc6c4 100644 --- a/apps/internal/logger/logger_121_below.go +++ b/apps/internal/logger/logger_121_below.go @@ -2,8 +2,6 @@ package logger -import "context" - type logger struct{} func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { @@ -11,7 +9,7 @@ func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { } // Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { +func (a *logger) Log(level Level, message string, fields ...any) { return } diff --git a/apps/internal/logger/logger_121_plus.go b/apps/internal/logger/logger_121_plus.go index e807cea3..8b6f29e8 100644 --- a/apps/internal/logger/logger_121_plus.go +++ b/apps/internal/logger/logger_121_plus.go @@ -27,7 +27,7 @@ func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { } // Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { +func (a *logger) Log(level Level, message string, fields ...any) { if a == nil || a.logging == nil { return } @@ -47,7 +47,7 @@ func (a *logger) Log(ctx context.Context, level Level, message string, fields .. // Log the entry with the message and fields a.logging.Log( - ctx, + context.Background(), slogLevel, message, fields..., diff --git a/apps/internal/logger/logger_test.go b/apps/internal/logger/logger_test.go index 508f1583..897ea59c 100644 --- a/apps/internal/logger/logger_test.go +++ b/apps/internal/logger/logger_test.go @@ -2,7 +2,6 @@ package logger import ( "bytes" - "context" "log/slog" "testing" ) @@ -22,10 +21,10 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { } // Log messages - logInstance.Log(context.Background(), Info, "This is an info message via slog.", Field("username", "john_doe"), slog.Int("age", 30)) - logInstance.Log(context.Background(), Err, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - logInstance.Log(context.Background(), Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - logInstance.Log(context.Background(), Debug, "This is a debug message via slog.", slog.String("module", "main")) + logInstance.Log(Info, "This is an info message via slog.", Field("username", "john_doe"), slog.Int("age", 30)) + logInstance.Log(Err, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + logInstance.Log(Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + logInstance.Log(Debug, "This is a debug message via slog.", slog.String("module", "main")) // Check the output output := buf.String() From dffa979f51ab1bdcdedfae85269b3826855fed70 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 6 Jan 2025 17:14:40 +0000 Subject: [PATCH 08/31] * Makes changes so WithLogger is only accessible for 1.21 and above --- apps/confidential/confidential.go | 12 ------------ apps/confidential/confidential_121_plus.go | 21 +++++++++++++++++++++ apps/internal/logger/logger_test.go | 2 ++ 3 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 apps/confidential/confidential_121_plus.go diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index d21edd8b..312190ac 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -257,18 +257,6 @@ type clientOptions struct { // Option is an optional argument to New(). type Option func(o *clientOptions) -//  WithLogger allows for a custom logger to be set. -func WithLogger(l interface{}) Option { - return func(o *clientOptions) { - logInstance, err := logger.New(l) - if err != nil { - fmt.Println("Error creating logger with slog:", err) - return - } - o.logger = logInstance - } -} - // WithCache provides an accessor that will read and write authentication data to an externally managed cache. func WithCache(accessor cache.ExportReplace) Option { return func(o *clientOptions) { diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go new file mode 100644 index 00000000..474116ca --- /dev/null +++ b/apps/confidential/confidential_121_plus.go @@ -0,0 +1,21 @@ +//go:build go1.21 + +package confidential + +import ( + "fmt" + + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" +) + +//  WithLogger allows for a custom logger to be set. +func WithLogger(l interface{}) Option { + return func(o *clientOptions) { + logInstance, err := logger.New(l) + if err != nil { + fmt.Println("Error creating logger with slog:", err) + return + } + o.logger = logInstance + } +} diff --git a/apps/internal/logger/logger_test.go b/apps/internal/logger/logger_test.go index 897ea59c..eeed5f75 100644 --- a/apps/internal/logger/logger_test.go +++ b/apps/internal/logger/logger_test.go @@ -1,3 +1,5 @@ +//go:build go1.21 + package logger import ( From aa59b0fdf26596ab1c1b38e009de65399a5e8cc9 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Tue, 7 Jan 2025 16:00:23 +0000 Subject: [PATCH 09/31] * Updates logic to create default logger instance and remove returning of error * Updates tests --- apps/confidential/confidential.go | 8 ++-- apps/confidential/confidential_121_plus.go | 12 ++---- apps/internal/logger/logger.go | 7 +++- apps/internal/logger/logger_121_below.go | 8 ++-- apps/internal/logger/logger_121_plus.go | 25 ++++++------ apps/internal/logger/logger_test.go | 44 ++++++++++++++-------- apps/tests/devapps/logger/logger_sample.go | 16 +++----- 7 files changed, 66 insertions(+), 54 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 312190ac..8924130f 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -320,10 +320,8 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client return Client{}, err } autoEnabledRegion := os.Getenv("MSAL_FORCE_REGION") - defaultLogger, err := logger.New(nil) - if err != nil { - return Client{}, err - } + defaultLogger := logger.New(nil) + opts := clientOptions{ authority: authority, // if the caller specified a token provider, it will handle all details of authentication, using Client only as a token cache @@ -352,7 +350,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true - opts.logger.Log(logger.Info, "Created confidential client", logger.Field("clientID", clientID)) + opts.logger.Log(context.Background(), logger.Info, "Created confidential client", logger.Field("clientID", clientID)) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go index 474116ca..58ada07d 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/confidential_121_plus.go @@ -3,19 +3,15 @@ package confidential import ( - "fmt" + "log/slog" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" ) -//  WithLogger allows for a custom logger to be set. -func WithLogger(l interface{}) Option { +// WithLogger allows for a custom logger to be set. +func WithLogger(l *slog.Logger) Option { return func(o *clientOptions) { - logInstance, err := logger.New(l) - if err != nil { - fmt.Println("Error creating logger with slog:", err) - return - } + logInstance := logger.NewLogger(l) o.logger = logInstance } } diff --git a/apps/internal/logger/logger.go b/apps/internal/logger/logger.go index cd8dbdee..d5c74e0a 100644 --- a/apps/internal/logger/logger.go +++ b/apps/internal/logger/logger.go @@ -1,5 +1,7 @@ package logger +import "context" + type Level string const ( @@ -11,9 +13,10 @@ const ( // LoggerInterface defines the methods that a logger should implement type LoggerInterface interface { - Log(level Level, message string, fields ...any) + Log(ctx context.Context, level Level, message string, fields ...any) } -func New(loggerInterface interface{}) (LoggerInterface, error) { +// New creates a new logger instance +func New(loggerInterface interface{}) LoggerInterface { return NewLogger(loggerInterface) } diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/logger/logger_121_below.go index a1bfc6c4..eeaac8f6 100644 --- a/apps/internal/logger/logger_121_below.go +++ b/apps/internal/logger/logger_121_below.go @@ -2,14 +2,16 @@ package logger +import "context" + type logger struct{} -func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { - return &logger{}, nil +func NewLogger(loggerInterface interface{}) LoggerInterface { + return &logger{} } // Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *logger) Log(level Level, message string, fields ...any) { +func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { return } diff --git a/apps/internal/logger/logger_121_plus.go b/apps/internal/logger/logger_121_plus.go index 8b6f29e8..4253a2da 100644 --- a/apps/internal/logger/logger_121_plus.go +++ b/apps/internal/logger/logger_121_plus.go @@ -4,8 +4,8 @@ package logger import ( "context" - "fmt" "log/slog" + "os" ) // logger struct for Go 1.21+ with full `slog` logging support. @@ -13,21 +13,24 @@ type logger struct { logging *slog.Logger } -// New creates a new logger instance -func NewLogger(loggerInterface interface{}) (LoggerInterface, error) { +// New creates a new logger instance for Go 1.21+ with full `slog` logging support. +// A default logger instance is provided if the loggerInterface is nil or there is an issue with type assertion of the loggerInterface +func NewLogger(loggerInterface interface{}) LoggerInterface { if loggerInterface == nil { - return &logger{logging: nil}, nil + // Provide a default logger instance + defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + return &logger{logging: defaultLogger} } - - if loggerInterface, ok := loggerInterface.(*slog.Logger); ok { - return &logger{logging: loggerInterface}, nil + if slogLogger, ok := loggerInterface.(*slog.Logger); ok { + return &logger{logging: slogLogger} } - - return nil, fmt.Errorf("invalid input for Go 1.21+; expected *slog.Logger") + // Handle the case where the type assertion fails + defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + return &logger{logging: defaultLogger} } // Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *logger) Log(level Level, message string, fields ...any) { +func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { if a == nil || a.logging == nil { return } @@ -47,7 +50,7 @@ func (a *logger) Log(level Level, message string, fields ...any) { // Log the entry with the message and fields a.logging.Log( - context.Background(), + ctx, slogLevel, message, fields..., diff --git a/apps/internal/logger/logger_test.go b/apps/internal/logger/logger_test.go index eeed5f75..b9035a51 100644 --- a/apps/internal/logger/logger_test.go +++ b/apps/internal/logger/logger_test.go @@ -4,6 +4,7 @@ package logger import ( "bytes" + "context" "log/slog" "testing" ) @@ -17,16 +18,13 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { // Create a new logger instance slogLogger := slog.New(handler) - logInstance, err := New(slogLogger) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } + logInstance := New(slogLogger) // Log messages - logInstance.Log(Info, "This is an info message via slog.", Field("username", "john_doe"), slog.Int("age", 30)) - logInstance.Log(Err, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - logInstance.Log(Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - logInstance.Log(Debug, "This is a debug message via slog.", slog.String("module", "main")) + logInstance.Log(context.Background(), Info, "This is an info message via slog.", Field("username", "john_doe"), slog.Int("age", 30)) + logInstance.Log(context.Background(), Err, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + logInstance.Log(context.Background(), Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + logInstance.Log(context.Background(), Debug, "This is a debug message via slog.", slog.String("module", "main")) // Check the output output := buf.String() @@ -44,14 +42,30 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { } } -// This test is to emulate what happens if the user has a go version < 1.21 -// In this case, they will not have access to slog and will need to pass nil to the New function -func TestLogger_New_NilLogger(t *testing.T) { - // Attempt to create a new logger instance with nil slog.Logger - logInstance, err := New(nil) - if err != nil { - t.Fatalf("expected no error, got %v", err) +func TestNewLogger_NilLoggerInterface(t *testing.T) { + // Test case where loggerInterface is nil + logInstance := NewLogger(nil) + if logInstance == nil { + t.Fatalf("expected non-nil logInstance, got nil") + } +} + +func TestNewLogger_ValidSlogLogger(t *testing.T) { + // Test case where loggerInterface is a valid *slog.Logger + var buf bytes.Buffer + handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{ + Level: slog.LevelDebug, + }) + slogLogger := slog.New(handler) + logInstance := NewLogger(slogLogger) + if logInstance == nil { + t.Fatalf("expected non-nil logInstance, got nil") } +} + +func TestNewLogger_InvalidLoggerInterface(t *testing.T) { + // Test case where loggerInterface is an invalid type + logInstance := NewLogger("invalid type") if logInstance == nil { t.Fatalf("expected non-nil logInstance, got nil") } diff --git a/apps/tests/devapps/logger/logger_sample.go b/apps/tests/devapps/logger/logger_sample.go index 9c536afd..dec390b3 100644 --- a/apps/tests/devapps/logger/logger_sample.go +++ b/apps/tests/devapps/logger/logger_sample.go @@ -1,7 +1,7 @@ package main import ( - "fmt" + "context" "log/slog" "os" @@ -12,14 +12,10 @@ func main() { // Test for Go 1.21+ handlerOptions := &slog.HandlerOptions{} slogLogger := slog.New(slog.NewTextHandler(os.Stdout, handlerOptions)) - logInstance, err := logger.New(slogLogger) - if err != nil { - fmt.Println("Error creating logger with slog:", err) - return - } + logInstance := logger.New(slogLogger) - logInstance.Log(logger.Info, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) - logInstance.Log(logger.Err, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - logInstance.Log(logger.Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - logInstance.Log(logger.Debug, "This is a debug message via slog.", slog.String("module", "main")) + logInstance.Log(context.Background(), logger.Info, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) + logInstance.Log(context.Background(), logger.Err, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + logInstance.Log(context.Background(), logger.Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + logInstance.Log(context.Background(), logger.Debug, "This is a debug message via slog.", slog.String("module", "main")) } From da841801817bf67be9fc7244572d7723d585b2d2 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 8 Jan 2025 00:11:19 +0000 Subject: [PATCH 10/31] * Updates to remove LoggerInterface --- apps/confidential/confidential.go | 2 +- apps/confidential/confidential_121_plus.go | 2 +- apps/internal/logger/logger.go | 22 -------- apps/internal/logger/logger_121_below.go | 23 ++++---- apps/internal/logger/logger_121_plus.go | 61 ++++++---------------- apps/internal/logger/logger_test.go | 26 ++++----- apps/tests/devapps/logger/logger_sample.go | 11 ++-- 7 files changed, 41 insertions(+), 106 deletions(-) delete mode 100644 apps/internal/logger/logger.go diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 8924130f..ad193cc0 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -251,7 +251,7 @@ type clientOptions struct { capabilities []string disableInstanceDiscovery, sendX5C bool httpClient ops.HTTPClient - logger logger.LoggerInterface + logger *logger.Logger } // Option is an optional argument to New(). diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go index 58ada07d..af133bd1 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/confidential_121_plus.go @@ -11,7 +11,7 @@ import ( // WithLogger allows for a custom logger to be set. func WithLogger(l *slog.Logger) Option { return func(o *clientOptions) { - logInstance := logger.NewLogger(l) + logInstance := logger.New(l) o.logger = logInstance } } diff --git a/apps/internal/logger/logger.go b/apps/internal/logger/logger.go deleted file mode 100644 index d5c74e0a..00000000 --- a/apps/internal/logger/logger.go +++ /dev/null @@ -1,22 +0,0 @@ -package logger - -import "context" - -type Level string - -const ( - Info Level = "info" - Err Level = "error" - Warn Level = "warn" - Debug Level = "debug" -) - -// LoggerInterface defines the methods that a logger should implement -type LoggerInterface interface { - Log(ctx context.Context, level Level, message string, fields ...any) -} - -// New creates a new logger instance -func New(loggerInterface interface{}) LoggerInterface { - return NewLogger(loggerInterface) -} diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/logger/logger_121_below.go index eeaac8f6..e690b01f 100644 --- a/apps/internal/logger/logger_121_below.go +++ b/apps/internal/logger/logger_121_below.go @@ -1,21 +1,18 @@ -//go:build go1.18 && !go1.21 +//go:build !go1.21 package logger import "context" -type logger struct{} +type Level int -func NewLogger(loggerInterface interface{}) LoggerInterface { - return &logger{} -} +type Logger struct{} -// Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { - return -} +const ( + LevelDebug Level = iota + LevelInfo + LevelWarn + LevelError +) -// Field creates a slog field for any value -func Field(key string, value any) any { - return "" -} +func (*Logger) Log(context.Context, Level, string, ...any) {} diff --git a/apps/internal/logger/logger_121_plus.go b/apps/internal/logger/logger_121_plus.go index 4253a2da..e23cfdfa 100644 --- a/apps/internal/logger/logger_121_plus.go +++ b/apps/internal/logger/logger_121_plus.go @@ -3,58 +3,29 @@ package logger import ( - "context" "log/slog" "os" ) -// logger struct for Go 1.21+ with full `slog` logging support. -type logger struct { - logging *slog.Logger -} +type Level = slog.Level -// New creates a new logger instance for Go 1.21+ with full `slog` logging support. -// A default logger instance is provided if the loggerInterface is nil or there is an issue with type assertion of the loggerInterface -func NewLogger(loggerInterface interface{}) LoggerInterface { - if loggerInterface == nil { - // Provide a default logger instance - defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, nil)) - return &logger{logging: defaultLogger} - } - if slogLogger, ok := loggerInterface.(*slog.Logger); ok { - return &logger{logging: slogLogger} - } - // Handle the case where the type assertion fails - defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, nil)) - return &logger{logging: defaultLogger} -} +const ( + Debug = slog.LevelDebug + Info = slog.LevelInfo + Warn = slog.LevelWarn + Error = slog.LevelError +) -// Log method for Go 1.21+ with full support for structured logging and multiple log levels. -func (a *logger) Log(ctx context.Context, level Level, message string, fields ...any) { - if a == nil || a.logging == nil { - return - } - var slogLevel slog.Level - switch level { - case Info: - slogLevel = slog.LevelInfo - case Err: - slogLevel = slog.LevelError - case Warn: - slogLevel = slog.LevelWarn - case Debug: - slogLevel = slog.LevelDebug - default: - slogLevel = slog.LevelInfo - } +type Logger = slog.Logger - // Log the entry with the message and fields - a.logging.Log( - ctx, - slogLevel, - message, - fields..., - ) +// New creates a new logger instance for Go 1.21+ with full `slog` logging support. +// If nil is provided a default logger instance is created. +func New(slogLogger *slog.Logger) *slog.Logger { + if slogLogger == nil { + defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{})) + return defaultLogger + } + return slogLogger } // Field creates a slog field for any value diff --git a/apps/internal/logger/logger_test.go b/apps/internal/logger/logger_test.go index b9035a51..ac010daa 100644 --- a/apps/internal/logger/logger_test.go +++ b/apps/internal/logger/logger_test.go @@ -18,13 +18,13 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { // Create a new logger instance slogLogger := slog.New(handler) - logInstance := New(slogLogger) + loggerInstance := New(slogLogger) // Log messages - logInstance.Log(context.Background(), Info, "This is an info message via slog.", Field("username", "john_doe"), slog.Int("age", 30)) - logInstance.Log(context.Background(), Err, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - logInstance.Log(context.Background(), Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - logInstance.Log(context.Background(), Debug, "This is a debug message via slog.", slog.String("module", "main")) + loggerInstance.Log(context.Background(), slog.LevelInfo, "This is an info message via slog.", slog.Any("username", "john_doe"), slog.Int("age", 30)) + loggerInstance.Log(context.Background(), slog.LevelError, "This is an error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + loggerInstance.Log(context.Background(), slog.LevelWarn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + loggerInstance.Log(context.Background(), slog.LevelDebug, "This is a debug message via slog.", slog.String("module", "main")) // Check the output output := buf.String() @@ -43,29 +43,21 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { } func TestNewLogger_NilLoggerInterface(t *testing.T) { - // Test case where loggerInterface is nil - logInstance := NewLogger(nil) + // Test case where slogLogger is nil + logInstance := New(nil) if logInstance == nil { t.Fatalf("expected non-nil logInstance, got nil") } } func TestNewLogger_ValidSlogLogger(t *testing.T) { - // Test case where loggerInterface is a valid *slog.Logger + // Test case where slogLogger is a valid *slog.Logger var buf bytes.Buffer handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{ Level: slog.LevelDebug, }) slogLogger := slog.New(handler) - logInstance := NewLogger(slogLogger) - if logInstance == nil { - t.Fatalf("expected non-nil logInstance, got nil") - } -} - -func TestNewLogger_InvalidLoggerInterface(t *testing.T) { - // Test case where loggerInterface is an invalid type - logInstance := NewLogger("invalid type") + logInstance := New(slogLogger) if logInstance == nil { t.Fatalf("expected non-nil logInstance, got nil") } diff --git a/apps/tests/devapps/logger/logger_sample.go b/apps/tests/devapps/logger/logger_sample.go index dec390b3..66295a5a 100644 --- a/apps/tests/devapps/logger/logger_sample.go +++ b/apps/tests/devapps/logger/logger_sample.go @@ -4,18 +4,15 @@ import ( "context" "log/slog" "os" - - "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" ) func main() { // Test for Go 1.21+ handlerOptions := &slog.HandlerOptions{} slogLogger := slog.New(slog.NewTextHandler(os.Stdout, handlerOptions)) - logInstance := logger.New(slogLogger) - logInstance.Log(context.Background(), logger.Info, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) - logInstance.Log(context.Background(), logger.Err, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - logInstance.Log(context.Background(), logger.Warn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - logInstance.Log(context.Background(), logger.Debug, "This is a debug message via slog.", slog.String("module", "main")) + slogLogger.Log(context.Background(), slog.LevelInfo, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) + slogLogger.Log(context.Background(), slog.LevelError, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) + slogLogger.Log(context.Background(), slog.LevelWarn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) + slogLogger.Log(context.Background(), slog.LevelDebug, "This is a debug message via slog.", slog.String("module", "main")) } From 435bbb601eb94ca87d01d1b17b8b3e4f4c5f04aa Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 8 Jan 2025 11:40:19 +0000 Subject: [PATCH 11/31] * Adds missing functions to help compile go1.18 --- apps/internal/logger/logger_121_below.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/logger/logger_121_below.go index e690b01f..f6f5e5a9 100644 --- a/apps/internal/logger/logger_121_below.go +++ b/apps/internal/logger/logger_121_below.go @@ -9,10 +9,19 @@ type Level int type Logger struct{} const ( - LevelDebug Level = iota - LevelInfo - LevelWarn - LevelError + Debug Level = iota + Info + Warn + Error ) +// These are all noop functions for go < 1.21 +func New(logger *Logger) *Logger { + return &Logger{} +} + +func Field(key string, value any) any { + return nil +} + func (*Logger) Log(context.Context, Level, string, ...any) {} From 1a2ccbfb8f042ec5a8c9142b80f9c13ea29cc8c8 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 8 Jan 2025 16:24:32 +0000 Subject: [PATCH 12/31] * Updates logger to remove LoggerInterface --- apps/confidential/confidential.go | 3 +-- apps/confidential/confidential_121_plus.go | 3 +-- .../logger/{logger_121_plus.go => logger.go} | 18 ++++++++++++++---- apps/internal/logger/logger_121_below.go | 4 +++- 4 files changed, 19 insertions(+), 9 deletions(-) rename apps/internal/logger/{logger_121_plus.go => logger.go} (61%) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index ad193cc0..fb018420 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -320,7 +320,6 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client return Client{}, err } autoEnabledRegion := os.Getenv("MSAL_FORCE_REGION") - defaultLogger := logger.New(nil) opts := clientOptions{ authority: authority, @@ -328,7 +327,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, azureRegion: autoEnabledRegion, - logger: defaultLogger, + logger: nil, } for _, o := range options { o(&opts) diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go index af133bd1..c3c8ddac 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/confidential_121_plus.go @@ -11,7 +11,6 @@ import ( // WithLogger allows for a custom logger to be set. func WithLogger(l *slog.Logger) Option { return func(o *clientOptions) { - logInstance := logger.New(l) - o.logger = logInstance + o.logger = logger.New(l) } } diff --git a/apps/internal/logger/logger_121_plus.go b/apps/internal/logger/logger.go similarity index 61% rename from apps/internal/logger/logger_121_plus.go rename to apps/internal/logger/logger.go index e23cfdfa..2b0ee8bd 100644 --- a/apps/internal/logger/logger_121_plus.go +++ b/apps/internal/logger/logger.go @@ -3,6 +3,7 @@ package logger import ( + "context" "log/slog" "os" ) @@ -16,19 +17,28 @@ const ( Error = slog.LevelError ) -type Logger = slog.Logger +type Logger struct { + *slog.Logger +} // New creates a new logger instance for Go 1.21+ with full `slog` logging support. // If nil is provided a default logger instance is created. -func New(slogLogger *slog.Logger) *slog.Logger { +func New(slogLogger *slog.Logger) *Logger { if slogLogger == nil { defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{})) - return defaultLogger + return &Logger{defaultLogger} } - return slogLogger + return &Logger{slogLogger} } // Field creates a slog field for any value func Field(key string, value any) any { return slog.Any(key, value) } + +// Log logs a message with the given level and arguments +func (l *Logger) Log(ctx context.Context, level Level, msg string, args ...any) { + if l != nil { + l.Logger.Log(ctx, level, msg, args...) + } +} diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/logger/logger_121_below.go index f6f5e5a9..701008c2 100644 --- a/apps/internal/logger/logger_121_below.go +++ b/apps/internal/logger/logger_121_below.go @@ -24,4 +24,6 @@ func Field(key string, value any) any { return nil } -func (*Logger) Log(context.Context, Level, string, ...any) {} +func (l *Logger) Log(ctx context.Context, level Level, msg string, args ...any) { + // No-op +} From bc171810b9cff53370b31ea1234a224b3d84cde9 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Thu, 9 Jan 2025 19:44:31 +0000 Subject: [PATCH 13/31] * Updates packages to use slog instead of logger * Some comments resolved from PR --- apps/confidential/confidential.go | 7 ++- apps/confidential/confidential_121_plus.go | 14 +++--- apps/internal/logger/logger.go | 44 ------------------- apps/internal/slog/logger.go | 33 ++++++++++++++ .../{logger => slog}/logger_121_below.go | 4 +- apps/internal/{logger => slog}/logger_test.go | 26 +++++++---- 6 files changed, 63 insertions(+), 65 deletions(-) delete mode 100644 apps/internal/logger/logger.go create mode 100644 apps/internal/slog/logger.go rename apps/internal/{logger => slog}/logger_121_below.go (75%) rename apps/internal/{logger => slog}/logger_test.go (67%) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index fb018420..cfbdb758 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -24,13 +24,13 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/base" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/exported" - "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/accesstokens" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/authority" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/options" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/shared" + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/slog" ) /* @@ -251,7 +251,7 @@ type clientOptions struct { capabilities []string disableInstanceDiscovery, sendX5C bool httpClient ops.HTTPClient - logger *logger.Logger + logger slog.Logger } // Option is an optional argument to New(). @@ -327,7 +327,6 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, azureRegion: autoEnabledRegion, - logger: nil, } for _, o := range options { o(&opts) @@ -349,7 +348,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true - opts.logger.Log(context.Background(), logger.Info, "Created confidential client", logger.Field("clientID", clientID)) + opts.logger.Log(context.Background(), slog.LevelInfo, "Created confidential client", slog.Field("clientID", clientID)) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go index c3c8ddac..ddc00d17 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/confidential_121_plus.go @@ -3,14 +3,16 @@ package confidential import ( - "log/slog" - - "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/logger" + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/slog" ) -// WithLogger allows for a custom logger to be set. -func WithLogger(l *slog.Logger) Option { +// WithLogger allows for a custom slog logger to be set +// More information on slog here: https://pkg.go.dev/log/slog +// This can be used with confidential client when creating a New instance similar to this: +// customSlogLogger := slog.Default() +// confidentialClient, err := New(authority, clientID, cred, WithLogger(customSlogLogger)) +func WithLogger(l slog.Logger) Option { return func(o *clientOptions) { - o.logger = logger.New(l) + o.logger = l } } diff --git a/apps/internal/logger/logger.go b/apps/internal/logger/logger.go deleted file mode 100644 index 2b0ee8bd..00000000 --- a/apps/internal/logger/logger.go +++ /dev/null @@ -1,44 +0,0 @@ -//go:build go1.21 - -package logger - -import ( - "context" - "log/slog" - "os" -) - -type Level = slog.Level - -const ( - Debug = slog.LevelDebug - Info = slog.LevelInfo - Warn = slog.LevelWarn - Error = slog.LevelError -) - -type Logger struct { - *slog.Logger -} - -// New creates a new logger instance for Go 1.21+ with full `slog` logging support. -// If nil is provided a default logger instance is created. -func New(slogLogger *slog.Logger) *Logger { - if slogLogger == nil { - defaultLogger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{})) - return &Logger{defaultLogger} - } - return &Logger{slogLogger} -} - -// Field creates a slog field for any value -func Field(key string, value any) any { - return slog.Any(key, value) -} - -// Log logs a message with the given level and arguments -func (l *Logger) Log(ctx context.Context, level Level, msg string, args ...any) { - if l != nil { - l.Logger.Log(ctx, level, msg, args...) - } -} diff --git a/apps/internal/slog/logger.go b/apps/internal/slog/logger.go new file mode 100644 index 00000000..74ea8b93 --- /dev/null +++ b/apps/internal/slog/logger.go @@ -0,0 +1,33 @@ +//go:build go1.21 + +package slog + +import ( + "log/slog" +) + +type Level = slog.Level + +const ( + LevelDebug = slog.LevelDebug + LevelInfo = slog.LevelInfo + LevelWarn = slog.LevelWarn + LevelError = slog.LevelError +) + +type Logger = slog.Logger + +// New creates a new logger instance for Go 1.21+ with full `slog` logging support. +// If nil is provided a default logger instance is created. +func New(slogLogger *slog.Logger) *Logger { + if slogLogger == nil { + defaultLogger := slog.Default() + return defaultLogger + } + return slogLogger +} + +// Field creates a slog field for any value +func Field(key string, value any) any { + return slog.Any(key, value) +} diff --git a/apps/internal/logger/logger_121_below.go b/apps/internal/slog/logger_121_below.go similarity index 75% rename from apps/internal/logger/logger_121_below.go rename to apps/internal/slog/logger_121_below.go index 701008c2..7dd61d9b 100644 --- a/apps/internal/logger/logger_121_below.go +++ b/apps/internal/slog/logger_121_below.go @@ -1,6 +1,6 @@ //go:build !go1.21 -package logger +package slog import "context" @@ -24,6 +24,6 @@ func Field(key string, value any) any { return nil } -func (l *Logger) Log(ctx context.Context, level Level, msg string, args ...any) { +func (*Logger) Log(context.Context, Level, string, ...any) {} // No-op } diff --git a/apps/internal/logger/logger_test.go b/apps/internal/slog/logger_test.go similarity index 67% rename from apps/internal/logger/logger_test.go rename to apps/internal/slog/logger_test.go index ac010daa..ec56212c 100644 --- a/apps/internal/logger/logger_test.go +++ b/apps/internal/slog/logger_test.go @@ -1,6 +1,6 @@ //go:build go1.21 -package logger +package slog import ( "bytes" @@ -28,16 +28,24 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { // Check the output output := buf.String() - expectedMessages := []string{ - "This is an info message via slog.", - "This is an error message via slog.", - "This is a warn message via slog.", - "This is a debug message via slog.", + expectedMessages := []struct { + msg string + contains []string + }{ + {"This is an info message via slog.", []string{`"username":"john_doe"`, `"age":30}`}}, + {"This is an error message via slog.", []string{`"module":"user-service"`, `"retry":3}`}}, + {"This is a warn message via slog.", []string{`"free_space_mb":100}`}}, + {"This is a debug message via slog.", []string{`"module":"main"`}}, } - for _, msg := range expectedMessages { - if !bytes.Contains([]byte(output), []byte(msg)) { - t.Errorf("expected log message %q not found in output", msg) + for _, expected := range expectedMessages { + if !bytes.Contains([]byte(output), []byte(expected.msg)) { + t.Errorf("expected log message %q not found in output", expected.msg) + } + for _, attr := range expected.contains { + if !bytes.Contains([]byte(output), []byte(attr)) { + t.Errorf("expected attribute %q not found in output for message %q", attr, expected.msg) + } } } } From f4639c50de29c73a3414b4e12a66abcf59ba39a2 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Thu, 9 Jan 2025 20:03:21 +0000 Subject: [PATCH 14/31] * Updates to use pointer --- apps/confidential/confidential.go | 2 +- apps/confidential/confidential_121_plus.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index cfbdb758..ff5a7161 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -251,7 +251,7 @@ type clientOptions struct { capabilities []string disableInstanceDiscovery, sendX5C bool httpClient ops.HTTPClient - logger slog.Logger + logger *slog.Logger } // Option is an optional argument to New(). diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go index ddc00d17..97f77b87 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/confidential_121_plus.go @@ -11,7 +11,8 @@ import ( // This can be used with confidential client when creating a New instance similar to this: // customSlogLogger := slog.Default() // confidentialClient, err := New(authority, clientID, cred, WithLogger(customSlogLogger)) -func WithLogger(l slog.Logger) Option { +// Just to note that WithLogger(nil) == WithLogger(slog.Default()) +func WithLogger(l *slog.Logger) Option { return func(o *clientOptions) { o.logger = l } From fca7a8a03251e8faf1288b688a306856507feb06 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Thu, 9 Jan 2025 22:48:37 +0000 Subject: [PATCH 15/31] * update documentation --- apps/confidential/confidential_121_plus.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/confidential_121_plus.go index 97f77b87..b98bd68f 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/confidential_121_plus.go @@ -6,12 +6,9 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/slog" ) -// WithLogger allows for a custom slog logger to be set -// More information on slog here: https://pkg.go.dev/log/slog -// This can be used with confidential client when creating a New instance similar to this: -// customSlogLogger := slog.Default() -// confidentialClient, err := New(authority, clientID, cred, WithLogger(customSlogLogger)) -// Just to note that WithLogger(nil) == WithLogger(slog.Default()) +// WithLogger enables logging within the SDK +// When l is nil, client will use slog.Default() +// Panic will occur if slog.Default() returns nil func WithLogger(l *slog.Logger) Option { return func(o *clientOptions) { o.logger = l From 61b5358fcb705297341cc82652601ecf821424cc Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Thu, 9 Jan 2025 22:52:50 +0000 Subject: [PATCH 16/31] * Removed some uneeded fileds and information --- apps/confidential/confidential.go | 1 - apps/design/logging.md | 10 ---------- apps/tests/devapps/logger/logger_sample.go | 18 ------------------ 3 files changed, 29 deletions(-) delete mode 100644 apps/tests/devapps/logger/logger_sample.go diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index ff5a7161..4b32d256 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -348,7 +348,6 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true - opts.logger.Log(context.Background(), slog.LevelInfo, "Created confidential client", slog.Field("clientID", clientID)) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/design/logging.md b/apps/design/logging.md index e2bc6a94..49d38e9c 100644 --- a/apps/design/logging.md +++ b/apps/design/logging.md @@ -24,13 +24,3 @@ time=2024-12-19T01:25:58.730Z level=INFO msg="Default log message." module=main 3. **Structured Logging**: - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. - -### **Sample** - -A sample of the logger can be found in the following location: - -```plaintext -microsoft-authentication-library-for-go/apps/devapps -├── logger/ -│ ├── logger_sample.go -``` diff --git a/apps/tests/devapps/logger/logger_sample.go b/apps/tests/devapps/logger/logger_sample.go deleted file mode 100644 index 66295a5a..00000000 --- a/apps/tests/devapps/logger/logger_sample.go +++ /dev/null @@ -1,18 +0,0 @@ -package main - -import ( - "context" - "log/slog" - "os" -) - -func main() { - // Test for Go 1.21+ - handlerOptions := &slog.HandlerOptions{} - slogLogger := slog.New(slog.NewTextHandler(os.Stdout, handlerOptions)) - - slogLogger.Log(context.Background(), slog.LevelInfo, "This is a info message via slog.", slog.String("username", "john_doe"), slog.Int("age", 30)) - slogLogger.Log(context.Background(), slog.LevelError, "This is a error message via slog.", slog.String("module", "user-service"), slog.Int("retry", 3)) - slogLogger.Log(context.Background(), slog.LevelWarn, "This is a warn message via slog.", slog.Int("free_space_mb", 100)) - slogLogger.Log(context.Background(), slog.LevelDebug, "This is a debug message via slog.", slog.String("module", "main")) -} From c9b47f5fc274ff5da3dafbf0f23e357c1ec548af Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 13 Jan 2025 11:04:13 +0000 Subject: [PATCH 17/31] * More PR Changes --- .../{confidential_121_plus.go => with_logger.go} | 2 +- apps/internal/slog/logger_121_below.go | 8 ++++---- apps/internal/slog/logger_test.go | 10 ++++++++-- 3 files changed, 13 insertions(+), 7 deletions(-) rename apps/confidential/{confidential_121_plus.go => with_logger.go} (78%) diff --git a/apps/confidential/confidential_121_plus.go b/apps/confidential/with_logger.go similarity index 78% rename from apps/confidential/confidential_121_plus.go rename to apps/confidential/with_logger.go index b98bd68f..134ca0a8 100644 --- a/apps/confidential/confidential_121_plus.go +++ b/apps/confidential/with_logger.go @@ -3,7 +3,7 @@ package confidential import ( - "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/slog" + "log/slog" ) // WithLogger enables logging within the SDK diff --git a/apps/internal/slog/logger_121_below.go b/apps/internal/slog/logger_121_below.go index 7dd61d9b..f1f7c731 100644 --- a/apps/internal/slog/logger_121_below.go +++ b/apps/internal/slog/logger_121_below.go @@ -9,10 +9,10 @@ type Level int type Logger struct{} const ( - Debug Level = iota - Info - Warn - Error + LevelDebug Level = iota + LevelInfo + LevelWarn + LevelError ) // These are all noop functions for go < 1.21 diff --git a/apps/internal/slog/logger_test.go b/apps/internal/slog/logger_test.go index ec56212c..c6eca687 100644 --- a/apps/internal/slog/logger_test.go +++ b/apps/internal/slog/logger_test.go @@ -50,12 +50,18 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { } } -func TestNewLogger_NilLoggerInterface(t *testing.T) { - // Test case where slogLogger is nil +func TestNewLogger_NilLogger_Returns_Default(t *testing.T) { + testLogger := slog.New(slog.Default().Handler()) + slog.SetDefault(testLogger) + logInstance := New(nil) if logInstance == nil { t.Fatalf("expected non-nil logInstance, got nil") } + + if logInstance != testLogger { + t.Fatalf("expected logInstance to be the default logger, got different logger") + } } func TestNewLogger_ValidSlogLogger(t *testing.T) { From 6d69a311a8154a9fd8aa9db3cf3136d0d26d34f2 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Fri, 17 Jan 2025 13:16:54 +0000 Subject: [PATCH 18/31] Changes to how logger works --- apps/confidential/confidential.go | 3 +++ apps/confidential/confidential_test.go | 7 ++++++- apps/internal/slog/logger.go | 25 +++++++++++++++++-------- apps/internal/slog/logger_121_below.go | 16 ++++++++++------ 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 4b32d256..5d8e9c48 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -327,6 +327,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, azureRegion: autoEnabledRegion, + logger: slog.New(&slog.NopHandler{}), } for _, o := range options { o(&opts) @@ -348,6 +349,7 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true + // opts.logger.Log(context.Background(), slog.LevelInfo, "confidential.Client.New", slog.Any("one", "Created confidential client")) return Client{base: base, cred: internalCred}, nil } @@ -376,6 +378,7 @@ func (cca Client) AuthCodeURL(ctx context.Context, clientID, redirectURI string, ap.Claims = o.claims ap.LoginHint = o.loginHint ap.DomainHint = o.domainHint + return cca.base.AuthCodeURL(ctx, clientID, redirectURI, scopes, ap) } diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index 9a9c9ee1..c2ba97a1 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -13,6 +13,10 @@ import ( "errors" "fmt" "io" + "log/slog" + + // "log/slog" + "net/http" "net/url" "os" @@ -185,7 +189,8 @@ func TestRegionAutoEnable_EmptyRegion_EnvRegion(t *testing.T) { lmo := "login.microsoftonline.com" tenant := "tenant" mockClient := mock.Client{} - client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient)) + slogLogger := slog.Default() + client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient), WithLogger(slogLogger)) if err != nil { t.Fatal(err) } diff --git a/apps/internal/slog/logger.go b/apps/internal/slog/logger.go index 74ea8b93..ccdefee8 100644 --- a/apps/internal/slog/logger.go +++ b/apps/internal/slog/logger.go @@ -3,6 +3,7 @@ package slog import ( + "context" "log/slog" ) @@ -15,19 +16,27 @@ const ( LevelError = slog.LevelError ) +type Handler = slog.Handler + type Logger = slog.Logger +type NopHandler struct{} + +func (*NopHandler) Enabled(context.Context, slog.Level) bool { return false } + +func (*NopHandler) Handle(context.Context, slog.Record) error { return nil } + +func (h *NopHandler) WithAttrs([]slog.Attr) slog.Handler { return h } + +func (h *NopHandler) WithGroup(string) slog.Handler { return h } + // New creates a new logger instance for Go 1.21+ with full `slog` logging support. // If nil is provided a default logger instance is created. -func New(slogLogger *slog.Logger) *Logger { - if slogLogger == nil { - defaultLogger := slog.Default() - return defaultLogger - } - return slogLogger +func New(h Handler) *Logger { + return slog.New(h) } -// Field creates a slog field for any value -func Field(key string, value any) any { +// Any creates a slog field for any value +func Any(key string, value any) any { return slog.Any(key, value) } diff --git a/apps/internal/slog/logger_121_below.go b/apps/internal/slog/logger_121_below.go index f1f7c731..40d9cd4e 100644 --- a/apps/internal/slog/logger_121_below.go +++ b/apps/internal/slog/logger_121_below.go @@ -10,20 +10,24 @@ type Logger struct{} const ( LevelDebug Level = iota - LevelInfo - LevelWarn + LevelInfo + LevelWarn LevelError ) +type NopHandler struct{} + // These are all noop functions for go < 1.21 -func New(logger *Logger) *Logger { +func New(h any) *Logger { return &Logger{} } +// func New(logger *Logger) *Logger { +// return &Logger{} +// } + func Field(key string, value any) any { return nil } -func (*Logger) Log(context.Context, Level, string, ...any) {} - // No-op -} +func (*Logger) Log(ctx context.Context, level Level, msg string, args ...any) {} From 41d9c771e90d76f6d9da9f4e7f5a69b08e588aaf Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 20 Jan 2025 14:09:19 +0000 Subject: [PATCH 19/31] * Adds logger to managed identity * Adds tests * Removes from confidential client --- apps/confidential/confidential.go | 4 - apps/confidential/confidential_test.go | 5 +- apps/internal/slog/logger.go | 17 ++- apps/internal/slog/logger_121_below.go | 12 +- apps/internal/slog/logger_test.go | 28 +--- apps/managedidentity/managedidentity.go | 39 ++++-- apps/managedidentity/managedidentity_test.go | 1 + .../managedidentity_with_logger_test.go | 128 ++++++++++++++++++ .../with_logger.go | 6 +- 9 files changed, 183 insertions(+), 57 deletions(-) create mode 100644 apps/managedidentity/managedidentity_with_logger_test.go rename apps/{confidential => managedidentity}/with_logger.go (66%) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index 5d8e9c48..d3100a7e 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -30,7 +30,6 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/authority" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/options" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/shared" - "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/slog" ) /* @@ -251,7 +250,6 @@ type clientOptions struct { capabilities []string disableInstanceDiscovery, sendX5C bool httpClient ops.HTTPClient - logger *slog.Logger } // Option is an optional argument to New(). @@ -327,7 +325,6 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client disableInstanceDiscovery: cred.tokenProvider != nil, httpClient: shared.DefaultClient, azureRegion: autoEnabledRegion, - logger: slog.New(&slog.NopHandler{}), } for _, o := range options { o(&opts) @@ -349,7 +346,6 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client } base.AuthParams.IsConfidentialClient = true - // opts.logger.Log(context.Background(), slog.LevelInfo, "confidential.Client.New", slog.Any("one", "Created confidential client")) return Client{base: base, cred: internalCred}, nil } diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index 70a74dac..cccc59b7 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -13,7 +13,6 @@ import ( "errors" "fmt" "io" - "log/slog" // "log/slog" @@ -190,8 +189,8 @@ func TestRegionAutoEnable_EmptyRegion_EnvRegion(t *testing.T) { lmo := "login.microsoftonline.com" tenant := "tenant" mockClient := mock.Client{} - slogLogger := slog.Default() - client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient), WithLogger(slogLogger)) + + client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient)) if err != nil { t.Fatal(err) } diff --git a/apps/internal/slog/logger.go b/apps/internal/slog/logger.go index ccdefee8..94e592da 100644 --- a/apps/internal/slog/logger.go +++ b/apps/internal/slog/logger.go @@ -17,21 +17,15 @@ const ( ) type Handler = slog.Handler - type Logger = slog.Logger - type NopHandler struct{} -func (*NopHandler) Enabled(context.Context, slog.Level) bool { return false } - +func (*NopHandler) Enabled(context.Context, slog.Level) bool { return false } func (*NopHandler) Handle(context.Context, slog.Record) error { return nil } - -func (h *NopHandler) WithAttrs([]slog.Attr) slog.Handler { return h } - -func (h *NopHandler) WithGroup(string) slog.Handler { return h } +func (h *NopHandler) WithAttrs([]slog.Attr) slog.Handler { return h } +func (h *NopHandler) WithGroup(string) slog.Handler { return h } // New creates a new logger instance for Go 1.21+ with full `slog` logging support. -// If nil is provided a default logger instance is created. func New(h Handler) *Logger { return slog.New(h) } @@ -40,3 +34,8 @@ func New(h Handler) *Logger { func Any(key string, value any) any { return slog.Any(key, value) } + +// String creates a slog field for a string value +func String(key, value string) slog.Attr { + return slog.String(key, value) +} diff --git a/apps/internal/slog/logger_121_below.go b/apps/internal/slog/logger_121_below.go index 40d9cd4e..5787e3aa 100644 --- a/apps/internal/slog/logger_121_below.go +++ b/apps/internal/slog/logger_121_below.go @@ -2,7 +2,9 @@ package slog -import "context" +import ( + "context" +) type Level int @@ -22,11 +24,11 @@ func New(h any) *Logger { return &Logger{} } -// func New(logger *Logger) *Logger { -// return &Logger{} -// } +func Any(key string, value any) any { + return nil +} -func Field(key string, value any) any { +func String(key, value string) any { return nil } diff --git a/apps/internal/slog/logger_test.go b/apps/internal/slog/logger_test.go index c6eca687..bbfdf0d1 100644 --- a/apps/internal/slog/logger_test.go +++ b/apps/internal/slog/logger_test.go @@ -12,13 +12,14 @@ import ( func TestLogger_Log_ConsoleOutput(t *testing.T) { // Capture the console output var buf bytes.Buffer + + // Create a new JSON handler handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{ Level: slog.LevelDebug, // Set the log level to Debug to capture all log levels }) - // Create a new logger instance - slogLogger := slog.New(handler) - loggerInstance := New(slogLogger) + // Create a new logger instance with the handler + loggerInstance := New(handler) // Log messages loggerInstance.Log(context.Background(), slog.LevelInfo, "This is an info message via slog.", slog.Any("username", "john_doe"), slog.Int("age", 30)) @@ -50,28 +51,13 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { } } -func TestNewLogger_NilLogger_Returns_Default(t *testing.T) { - testLogger := slog.New(slog.Default().Handler()) - slog.SetDefault(testLogger) - - logInstance := New(nil) - if logInstance == nil { - t.Fatalf("expected non-nil logInstance, got nil") - } - - if logInstance != testLogger { - t.Fatalf("expected logInstance to be the default logger, got different logger") - } -} - -func TestNewLogger_ValidSlogLogger(t *testing.T) { - // Test case where slogLogger is a valid *slog.Logger +func TestNewLogger_ValidSlogHandler(t *testing.T) { + // Test case where handler is a valid slog.Handler var buf bytes.Buffer handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{ Level: slog.LevelDebug, }) - slogLogger := slog.New(handler) - logInstance := New(slogLogger) + logInstance := New(handler) if logInstance == nil { t.Fatalf("expected non-nil logInstance, got nil") } diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index 4193ed60..88966feb 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -29,6 +29,7 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/accesstokens" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/authority" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/shared" + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/slog" ) const ( @@ -158,11 +159,13 @@ type Client struct { source Source authParams authority.AuthParams retryPolicyEnabled bool + logger *slog.Logger } type ClientOptions struct { httpClient ops.HTTPClient retryPolicyEnabled bool + logger *slog.Logger } type AcquireTokenOptions struct { @@ -197,8 +200,17 @@ func WithRetryPolicyDisabled() ClientOption { // Client to be used to acquire tokens for managed identity. // ID: [SystemAssigned], [UserAssignedClientID], [UserAssignedResourceID], [UserAssignedObjectID] // -// Options: [WithHTTPClient] +// Options: [WithHTTPClient], [WithLogger] func New(id ID, options ...ClientOption) (Client, error) { + opts := ClientOptions{ + httpClient: shared.DefaultClient, + retryPolicyEnabled: true, + logger: slog.New(&slog.NopHandler{}), + } + for _, option := range options { + option(&opts) + } + source, err := GetSource() if err != nil { return Client{}, err @@ -211,13 +223,6 @@ func New(id ID, options ...ClientOption) (Client, error) { return Client{}, errors.New("azure Arc doesn't support user assigned managed identities") } } - opts := ClientOptions{ - httpClient: shared.DefaultClient, - retryPolicyEnabled: true, - } - for _, option := range options { - option(&opts) - } switch t := id.(type) { case UserAssignedClientID: @@ -236,17 +241,22 @@ func New(id ID, options ...ClientOption) (Client, error) { default: return Client{}, fmt.Errorf("unsupported type %T", id) } + client := Client{ miType: id, httpClient: opts.httpClient, retryPolicyEnabled: opts.retryPolicyEnabled, source: source, + logger: opts.logger, } + fakeAuthInfo, err := authority.NewInfoFromAuthorityURI("https://login.microsoftonline.com/managed_identity", false, true) if err != nil { return Client{}, err } + client.authParams = authority.NewAuthParams(client.miType.value(), fakeAuthInfo) + return client, nil } @@ -282,9 +292,11 @@ func (c Client) AcquireToken(ctx context.Context, resource string, options ...Ac for _, option := range options { option(&o) } + c.authParams.Scopes = []string{resource} + c.logger.Log(ctx, slog.LevelInfo, "Managed Identity", slog.String("source", string(c.source))) - // ignore cached access tokens when given claims + // when claims are empty, we get token from the cache, otherwise acquire a new one if o.claims == "" { storageTokenResponse, err := cacheManager.Read(ctx, c.authParams) if err != nil { @@ -296,6 +308,7 @@ func (c Client) AcquireToken(ctx context.Context, resource string, options ...Ac return ar, err } } + switch c.source { case AzureArc: return c.acquireTokenForAzureArc(ctx, resource) @@ -397,6 +410,8 @@ func (c Client) retry(maxRetries int, req *http.Request) (*http.Response, error) var err error for attempt := 0; attempt < maxRetries; attempt++ { tryCtx, tryCancel := context.WithTimeout(req.Context(), time.Minute) + c.logger.Log(tryCtx, slog.LevelInfo, "Managed Identity retrying request", slog.String("attempt", fmt.Sprint(attempt))) + defer tryCancel() if resp != nil && resp.Body != nil { _, _ = io.Copy(io.Discard, resp.Body) @@ -404,11 +419,11 @@ func (c Client) retry(maxRetries int, req *http.Request) (*http.Response, error) } cloneReq := req.Clone(tryCtx) resp, err = c.httpClient.Do(cloneReq) - retrylist := retryStatusCodes + retryList := retryStatusCodes if c.source == DefaultToIMDS { - retrylist = retryCodesForIMDS + retryList = retryCodesForIMDS } - if err == nil && !contains(retrylist, resp.StatusCode) { + if err == nil && !contains(retryList, resp.StatusCode) { return resp, nil } select { diff --git a/apps/managedidentity/managedidentity_test.go b/apps/managedidentity/managedidentity_test.go index c5bbe3d2..df8b501b 100644 --- a/apps/managedidentity/managedidentity_test.go +++ b/apps/managedidentity/managedidentity_test.go @@ -663,6 +663,7 @@ func TestAzureArcOnlySystemAssignedSupported(t *testing.T) { } } + func TestAzureArcPlatformSupported(t *testing.T) { setEnvVars(t, AzureArc) setCustomAzureArcFilePath(t, fakeAzureArcFilePath) diff --git a/apps/managedidentity/managedidentity_with_logger_test.go b/apps/managedidentity/managedidentity_with_logger_test.go new file mode 100644 index 00000000..57c902f6 --- /dev/null +++ b/apps/managedidentity/managedidentity_with_logger_test.go @@ -0,0 +1,128 @@ +//go:build go1.21 + +package managedidentity + +import ( + "bytes" + "context" + "log/slog" + "net/http" + "strings" + "testing" + + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/mock" +) + +// Custom log handler to capture log output +type BufferHandler struct { + buf bytes.Buffer +} + +// Custom log handler to capture log output and filter out info level logs +type FilteredBufferHandler struct { + buf bytes.Buffer +} + +func (h *BufferHandler) Enabled(ctx context.Context, level slog.Level) bool { return true } +func (h *BufferHandler) Handle(ctx context.Context, record slog.Record) error { + h.buf.WriteString(record.Message + " ") + record.Attrs(func(attr slog.Attr) bool { + h.buf.WriteString(attr.Key + "=" + attr.Value.String() + " ") + return true + }) + return nil +} +func (h *BufferHandler) WithAttrs(attrs []slog.Attr) slog.Handler { return h } +func (h *BufferHandler) WithGroup(name string) slog.Handler { return h } +func (h *FilteredBufferHandler) Enabled(ctx context.Context, level slog.Level) bool { + return level == slog.LevelDebug +} +func (h *FilteredBufferHandler) Handle(ctx context.Context, record slog.Record) error { + h.buf.WriteString(record.Message + " ") + record.Attrs(func(attr slog.Attr) bool { + h.buf.WriteString(attr.Key + "=" + attr.Value.String() + " ") + return true + }) + return nil +} +func (h *FilteredBufferHandler) WithAttrs(attrs []slog.Attr) slog.Handler { return h } +func (h *FilteredBufferHandler) WithGroup(name string) slog.Handler { return h } + +func TestClientLogging(t *testing.T) { + // Set up mock client + mockClient := mock.Client{} + headers := http.Header{} + headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") + mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) + + // Create a custom logger with BufferHandler + bufferHandler := &BufferHandler{} + customLogger := slog.New(bufferHandler) + + client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger)) + if err != nil { + t.Fatal(err) + } + + // Call AcquireToken to trigger logging + _, _ = client.AcquireToken(context.Background(), "https://resource") + + // Verify the log output + logOutput := bufferHandler.buf.String() + expectedLogMessage := "Managed Identity" + if !strings.Contains(logOutput, expectedLogMessage) { + t.Errorf("expected log message %q not found in output", expectedLogMessage) + } +} + +func TestClientLogging_NoLoggerProvided(t *testing.T) { + // Set up mock client + mockClient := mock.Client{} + headers := http.Header{} + headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") + mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) + + // Create a custom logger with BufferHandler to capture logs + bufferHandler := &BufferHandler{} + + client, err := New(SystemAssigned(), WithHTTPClient(&mockClient)) + if err != nil { + t.Fatal(err) + } + + // Call AcquireToken to trigger logging + _, _ = client.AcquireToken(context.Background(), "https://resource") + + // Verify that no logs are captured + logOutput := bufferHandler.buf.String() + if logOutput != "" { + t.Errorf("expected no log output, but got: %q", logOutput) + } +} + +func TestClientLogging_CustomHandler(t *testing.T) { + // Set up mock client + mockClient := mock.Client{} + headers := http.Header{} + headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") + mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) + + // Create a custom logger with FilteredBufferHandler + filteredBufferHandler := &FilteredBufferHandler{} + customLogger := slog.New(filteredBufferHandler) + + client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger)) + if err != nil { + t.Fatal(err) + } + + // Call AcquireToken to trigger logging + _, _ = client.AcquireToken(context.Background(), "https://resource") + + // Verify the log output + logOutput := filteredBufferHandler.buf.String() + unexpectedLogMessage := "Managed Identity" + if strings.Contains(logOutput, unexpectedLogMessage) { + t.Errorf("unexpected log message %q found in output", unexpectedLogMessage) + } +} diff --git a/apps/confidential/with_logger.go b/apps/managedidentity/with_logger.go similarity index 66% rename from apps/confidential/with_logger.go rename to apps/managedidentity/with_logger.go index 134ca0a8..70dd59eb 100644 --- a/apps/confidential/with_logger.go +++ b/apps/managedidentity/with_logger.go @@ -1,6 +1,6 @@ //go:build go1.21 -package confidential +package managedidentity import ( "log/slog" @@ -9,8 +9,8 @@ import ( // WithLogger enables logging within the SDK // When l is nil, client will use slog.Default() // Panic will occur if slog.Default() returns nil -func WithLogger(l *slog.Logger) Option { - return func(o *clientOptions) { +func WithLogger(l *slog.Logger) ClientOption { + return func(o *ClientOptions) { o.logger = l } } From 281d3768ae5b13579ef4725f0ec4c51493f541af Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 22 Jan 2025 11:11:00 +0000 Subject: [PATCH 20/31] * Some small fixes such as extra lines and some comments being removed --- apps/confidential/confidential.go | 2 -- apps/confidential/confidential_test.go | 4 ---- apps/managedidentity/managedidentity.go | 6 ------ 3 files changed, 12 deletions(-) diff --git a/apps/confidential/confidential.go b/apps/confidential/confidential.go index d3100a7e..57d0e277 100644 --- a/apps/confidential/confidential.go +++ b/apps/confidential/confidential.go @@ -318,7 +318,6 @@ func New(authority, clientID string, cred Credential, options ...Option) (Client return Client{}, err } autoEnabledRegion := os.Getenv("MSAL_FORCE_REGION") - opts := clientOptions{ authority: authority, // if the caller specified a token provider, it will handle all details of authentication, using Client only as a token cache @@ -374,7 +373,6 @@ func (cca Client) AuthCodeURL(ctx context.Context, clientID, redirectURI string, ap.Claims = o.claims ap.LoginHint = o.loginHint ap.DomainHint = o.domainHint - return cca.base.AuthCodeURL(ctx, clientID, redirectURI, scopes, ap) } diff --git a/apps/confidential/confidential_test.go b/apps/confidential/confidential_test.go index cccc59b7..c91731ca 100644 --- a/apps/confidential/confidential_test.go +++ b/apps/confidential/confidential_test.go @@ -13,9 +13,6 @@ import ( "errors" "fmt" "io" - - // "log/slog" - "net/http" "net/url" "os" @@ -189,7 +186,6 @@ func TestRegionAutoEnable_EmptyRegion_EnvRegion(t *testing.T) { lmo := "login.microsoftonline.com" tenant := "tenant" mockClient := mock.Client{} - client, err := New(fmt.Sprintf(authorityFmt, lmo, tenant), fakeClientID, cred, WithHTTPClient(&mockClient)) if err != nil { t.Fatal(err) diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index 88966feb..47bd19ce 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -210,7 +210,6 @@ func New(id ID, options ...ClientOption) (Client, error) { for _, option := range options { option(&opts) } - source, err := GetSource() if err != nil { return Client{}, err @@ -241,7 +240,6 @@ func New(id ID, options ...ClientOption) (Client, error) { default: return Client{}, fmt.Errorf("unsupported type %T", id) } - client := Client{ miType: id, httpClient: opts.httpClient, @@ -254,9 +252,7 @@ func New(id ID, options ...ClientOption) (Client, error) { if err != nil { return Client{}, err } - client.authParams = authority.NewAuthParams(client.miType.value(), fakeAuthInfo) - return client, nil } @@ -292,7 +288,6 @@ func (c Client) AcquireToken(ctx context.Context, resource string, options ...Ac for _, option := range options { option(&o) } - c.authParams.Scopes = []string{resource} c.logger.Log(ctx, slog.LevelInfo, "Managed Identity", slog.String("source", string(c.source))) @@ -308,7 +303,6 @@ func (c Client) AcquireToken(ctx context.Context, resource string, options ...Ac return ar, err } } - switch c.source { case AzureArc: return c.acquireTokenForAzureArc(ctx, resource) From 0ef46fe44a002281fed969196ec283a56a31a318 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 22 Jan 2025 19:24:47 +0000 Subject: [PATCH 21/31] Update documentation WithLogger --- apps/managedidentity/with_logger.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/managedidentity/with_logger.go b/apps/managedidentity/with_logger.go index 70dd59eb..0ef4cc39 100644 --- a/apps/managedidentity/with_logger.go +++ b/apps/managedidentity/with_logger.go @@ -7,8 +7,6 @@ import ( ) // WithLogger enables logging within the SDK -// When l is nil, client will use slog.Default() -// Panic will occur if slog.Default() returns nil func WithLogger(l *slog.Logger) ClientOption { return func(o *ClientOptions) { o.logger = l From 7232018b3ae9df6bc9e99629d0b0d6c0fd6adc44 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 22 Jan 2025 19:32:50 +0000 Subject: [PATCH 22/31] Documentation update --- apps/design/logging.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/apps/design/logging.md b/apps/design/logging.md index 49d38e9c..51d68c42 100644 --- a/apps/design/logging.md +++ b/apps/design/logging.md @@ -2,7 +2,7 @@ GO 1.21 introduces enhanced features like structured logging with `log/slog`. As part of MSAL GO, we have decided to only support logging using slog from version 1.21. -The reason for this is `log/slog` greatly enhances the debugging and monitoring capabilities compared to the old SDK. This is especially useful in production environments where accurate, traceable logs are crucial for maintaining application health and troubleshooting issues. For versions older than 1.21, the user can specify *nil* to the *New()* function and a no-op is returned +The reason for this is `log/slog` greatly enhances the debugging and monitoring capabilities compared to the old SDK. This is especially useful in production environments where accurate, traceable logs are crucial for maintaining application health and troubleshooting issues. ## **Expected Output** @@ -15,12 +15,8 @@ time=2024-12-19T01:25:58.730Z level=INFO msg="Default log message." module=main ## Key Pointers -1. **For Go <= 1.20**: - - User should pass *nil* to `logger.New(nil)` - - No-op is returned and can be handled by user - -2. **Full `slog` Support for Go 1.21+**: +1. **Full `slog` Support for Go 1.21+**: - The `logger.go` file leverages the `slog` package, supporting structured logs, multiple log levels (`info`, `error`, `warn`), and fields. -3. **Structured Logging**: +2. **Structured Logging**: - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. From dee0a5d9bee815d6c53944fd1a9149a4bf5f5c0f Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Sun, 26 Jan 2025 23:38:35 +0000 Subject: [PATCH 23/31] Address PR comments --- apps/internal/slog/logger.go | 4 +- apps/internal/slog/logger_121_below.go | 5 +- apps/internal/slog/logger_test.go | 3 + apps/internal/slog/nop_handler.go | 6 ++ apps/managedidentity/managedidentity_test.go | 1 - .../managedidentity_with_logger_test.go | 60 ++----------------- 6 files changed, 21 insertions(+), 58 deletions(-) create mode 100644 apps/internal/slog/nop_handler.go diff --git a/apps/internal/slog/logger.go b/apps/internal/slog/logger.go index 94e592da..3ed6376e 100644 --- a/apps/internal/slog/logger.go +++ b/apps/internal/slog/logger.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + //go:build go1.21 package slog @@ -18,7 +21,6 @@ const ( type Handler = slog.Handler type Logger = slog.Logger -type NopHandler struct{} func (*NopHandler) Enabled(context.Context, slog.Level) bool { return false } func (*NopHandler) Handle(context.Context, slog.Record) error { return nil } diff --git a/apps/internal/slog/logger_121_below.go b/apps/internal/slog/logger_121_below.go index 5787e3aa..18cc1a78 100644 --- a/apps/internal/slog/logger_121_below.go +++ b/apps/internal/slog/logger_121_below.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + //go:build !go1.21 package slog @@ -17,8 +20,6 @@ const ( LevelError ) -type NopHandler struct{} - // These are all noop functions for go < 1.21 func New(h any) *Logger { return &Logger{} diff --git a/apps/internal/slog/logger_test.go b/apps/internal/slog/logger_test.go index bbfdf0d1..5920a8a5 100644 --- a/apps/internal/slog/logger_test.go +++ b/apps/internal/slog/logger_test.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + //go:build go1.21 package slog diff --git a/apps/internal/slog/nop_handler.go b/apps/internal/slog/nop_handler.go new file mode 100644 index 00000000..00f6e023 --- /dev/null +++ b/apps/internal/slog/nop_handler.go @@ -0,0 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package slog + +type NopHandler struct{} diff --git a/apps/managedidentity/managedidentity_test.go b/apps/managedidentity/managedidentity_test.go index df8b501b..c5bbe3d2 100644 --- a/apps/managedidentity/managedidentity_test.go +++ b/apps/managedidentity/managedidentity_test.go @@ -663,7 +663,6 @@ func TestAzureArcOnlySystemAssignedSupported(t *testing.T) { } } - func TestAzureArcPlatformSupported(t *testing.T) { setEnvVars(t, AzureArc) setCustomAzureArcFilePath(t, fakeAzureArcFilePath) diff --git a/apps/managedidentity/managedidentity_with_logger_test.go b/apps/managedidentity/managedidentity_with_logger_test.go index 57c902f6..2004656a 100644 --- a/apps/managedidentity/managedidentity_with_logger_test.go +++ b/apps/managedidentity/managedidentity_with_logger_test.go @@ -13,17 +13,15 @@ import ( "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/mock" ) -// Custom log handler to capture log output type BufferHandler struct { - buf bytes.Buffer + buf bytes.Buffer + level slog.Level } -// Custom log handler to capture log output and filter out info level logs -type FilteredBufferHandler struct { - buf bytes.Buffer +func (h *BufferHandler) Enabled(ctx context.Context, level slog.Level) bool { + return level <= h.level } -func (h *BufferHandler) Enabled(ctx context.Context, level slog.Level) bool { return true } func (h *BufferHandler) Handle(ctx context.Context, record slog.Record) error { h.buf.WriteString(record.Message + " ") record.Attrs(func(attr slog.Attr) bool { @@ -32,30 +30,15 @@ func (h *BufferHandler) Handle(ctx context.Context, record slog.Record) error { }) return nil } + func (h *BufferHandler) WithAttrs(attrs []slog.Attr) slog.Handler { return h } func (h *BufferHandler) WithGroup(name string) slog.Handler { return h } -func (h *FilteredBufferHandler) Enabled(ctx context.Context, level slog.Level) bool { - return level == slog.LevelDebug -} -func (h *FilteredBufferHandler) Handle(ctx context.Context, record slog.Record) error { - h.buf.WriteString(record.Message + " ") - record.Attrs(func(attr slog.Attr) bool { - h.buf.WriteString(attr.Key + "=" + attr.Value.String() + " ") - return true - }) - return nil -} -func (h *FilteredBufferHandler) WithAttrs(attrs []slog.Attr) slog.Handler { return h } -func (h *FilteredBufferHandler) WithGroup(name string) slog.Handler { return h } func TestClientLogging(t *testing.T) { - // Set up mock client mockClient := mock.Client{} headers := http.Header{} headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) - - // Create a custom logger with BufferHandler bufferHandler := &BufferHandler{} customLogger := slog.New(bufferHandler) @@ -64,10 +47,8 @@ func TestClientLogging(t *testing.T) { t.Fatal(err) } - // Call AcquireToken to trigger logging _, _ = client.AcquireToken(context.Background(), "https://resource") - // Verify the log output logOutput := bufferHandler.buf.String() expectedLogMessage := "Managed Identity" if !strings.Contains(logOutput, expectedLogMessage) { @@ -75,40 +56,13 @@ func TestClientLogging(t *testing.T) { } } -func TestClientLogging_NoLoggerProvided(t *testing.T) { - // Set up mock client - mockClient := mock.Client{} - headers := http.Header{} - headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") - mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) - - // Create a custom logger with BufferHandler to capture logs - bufferHandler := &BufferHandler{} - - client, err := New(SystemAssigned(), WithHTTPClient(&mockClient)) - if err != nil { - t.Fatal(err) - } - - // Call AcquireToken to trigger logging - _, _ = client.AcquireToken(context.Background(), "https://resource") - - // Verify that no logs are captured - logOutput := bufferHandler.buf.String() - if logOutput != "" { - t.Errorf("expected no log output, but got: %q", logOutput) - } -} - func TestClientLogging_CustomHandler(t *testing.T) { - // Set up mock client mockClient := mock.Client{} headers := http.Header{} headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) - // Create a custom logger with FilteredBufferHandler - filteredBufferHandler := &FilteredBufferHandler{} + filteredBufferHandler := &BufferHandler{level: slog.LevelDebug} customLogger := slog.New(filteredBufferHandler) client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger)) @@ -116,10 +70,8 @@ func TestClientLogging_CustomHandler(t *testing.T) { t.Fatal(err) } - // Call AcquireToken to trigger logging _, _ = client.AcquireToken(context.Background(), "https://resource") - // Verify the log output logOutput := filteredBufferHandler.buf.String() unexpectedLogMessage := "Managed Identity" if strings.Contains(logOutput, unexpectedLogMessage) { From 2f88c407e2ee17bce2adc7fd7d9353debe263b07 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 27 Jan 2025 10:45:59 +0000 Subject: [PATCH 24/31] * Add ability to pass WithPiiLogging --- apps/managedidentity/managedidentity.go | 4 ++++ .../managedidentity_with_funcs.go | 21 +++++++++++++++++++ apps/managedidentity/with_logger.go | 14 ------------- 3 files changed, 25 insertions(+), 14 deletions(-) create mode 100644 apps/managedidentity/managedidentity_with_funcs.go delete mode 100644 apps/managedidentity/with_logger.go diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index 47bd19ce..285ef720 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -160,12 +160,14 @@ type Client struct { authParams authority.AuthParams retryPolicyEnabled bool logger *slog.Logger + piiLogging bool } type ClientOptions struct { httpClient ops.HTTPClient retryPolicyEnabled bool logger *slog.Logger + piiLogging bool } type AcquireTokenOptions struct { @@ -206,6 +208,7 @@ func New(id ID, options ...ClientOption) (Client, error) { httpClient: shared.DefaultClient, retryPolicyEnabled: true, logger: slog.New(&slog.NopHandler{}), + piiLogging: false, } for _, option := range options { option(&opts) @@ -246,6 +249,7 @@ func New(id ID, options ...ClientOption) (Client, error) { retryPolicyEnabled: opts.retryPolicyEnabled, source: source, logger: opts.logger, + piiLogging: opts.piiLogging, } fakeAuthInfo, err := authority.NewInfoFromAuthorityURI("https://login.microsoftonline.com/managed_identity", false, true) diff --git a/apps/managedidentity/managedidentity_with_funcs.go b/apps/managedidentity/managedidentity_with_funcs.go new file mode 100644 index 00000000..d3d6beba --- /dev/null +++ b/apps/managedidentity/managedidentity_with_funcs.go @@ -0,0 +1,21 @@ +//go:build go1.21 + +package managedidentity + +import ( + "log/slog" +) + +// WithLogger enables logging within the SDK +func WithLogger(l *slog.Logger) ClientOption { + return func(o *ClientOptions) { + o.logger = l + } +} + +// WithPiiLogging enables logging of Personally Identifiable Information (PII) within the SDK +func WithPiiLogging(enablePiiLogging bool) ClientOption { + return func(o *ClientOptions) { + o.piiLogging = enablePiiLogging + } +} diff --git a/apps/managedidentity/with_logger.go b/apps/managedidentity/with_logger.go deleted file mode 100644 index 0ef4cc39..00000000 --- a/apps/managedidentity/with_logger.go +++ /dev/null @@ -1,14 +0,0 @@ -//go:build go1.21 - -package managedidentity - -import ( - "log/slog" -) - -// WithLogger enables logging within the SDK -func WithLogger(l *slog.Logger) ClientOption { - return func(o *ClientOptions) { - o.logger = l - } -} From 21ab563816690031d7085c34657b3dba0edc45f5 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 27 Jan 2025 10:47:32 +0000 Subject: [PATCH 25/31] * Update documentation --- apps/managedidentity/managedidentity.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index 285ef720..d9da3c34 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -202,7 +202,7 @@ func WithRetryPolicyDisabled() ClientOption { // Client to be used to acquire tokens for managed identity. // ID: [SystemAssigned], [UserAssignedClientID], [UserAssignedResourceID], [UserAssignedObjectID] // -// Options: [WithHTTPClient], [WithLogger] +// Options: [WithHTTPClient], [WithLogger], [WithPiiLogging] func New(id ID, options ...ClientOption) (Client, error) { opts := ClientOptions{ httpClient: shared.DefaultClient, From 6a8d62100a9e1386461aa05ac206e48df2d703c3 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Mon, 27 Jan 2025 10:55:56 +0000 Subject: [PATCH 26/31] * Update documentation * Remove bool from WithPiiLogging --- apps/design/logging.md | 3 +++ apps/managedidentity/managedidentity_with_funcs.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/design/logging.md b/apps/design/logging.md index 51d68c42..71fa732e 100644 --- a/apps/design/logging.md +++ b/apps/design/logging.md @@ -20,3 +20,6 @@ time=2024-12-19T01:25:58.730Z level=INFO msg="Default log message." module=main 2. **Structured Logging**: - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. +5rf +3. **PII Logging (Personally Identifiable Information)** + - You can allow for Pii logging in the SDK by using WithPiiLogging() when creating the client. This defaults to false diff --git a/apps/managedidentity/managedidentity_with_funcs.go b/apps/managedidentity/managedidentity_with_funcs.go index d3d6beba..bd364ea0 100644 --- a/apps/managedidentity/managedidentity_with_funcs.go +++ b/apps/managedidentity/managedidentity_with_funcs.go @@ -14,8 +14,8 @@ func WithLogger(l *slog.Logger) ClientOption { } // WithPiiLogging enables logging of Personally Identifiable Information (PII) within the SDK -func WithPiiLogging(enablePiiLogging bool) ClientOption { +func WithPiiLogging() ClientOption { return func(o *ClientOptions) { - o.piiLogging = enablePiiLogging + o.piiLogging = true } } From f28417194f1d2178ab9e31f32d4ea690030df7a4 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Tue, 28 Jan 2025 15:19:29 +0000 Subject: [PATCH 27/31] * Removes separate WithPiiLogging to add bool to WithLogger --- apps/design/logging.md | 2 +- apps/managedidentity/managedidentity.go | 2 +- apps/managedidentity/managedidentity_with_funcs.go | 11 +++-------- .../managedidentity_with_logger_test.go | 4 ++-- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/apps/design/logging.md b/apps/design/logging.md index 71fa732e..187be23e 100644 --- a/apps/design/logging.md +++ b/apps/design/logging.md @@ -22,4 +22,4 @@ time=2024-12-19T01:25:58.730Z level=INFO msg="Default log message." module=main - You can pass key-value pairs using `slog.String`, `slog.Int`, etc., for structured logging, which is handled by `slog` in Go 1.21 and later. 5rf 3. **PII Logging (Personally Identifiable Information)** - - You can allow for Pii logging in the SDK by using WithPiiLogging() when creating the client. This defaults to false + - You can allow for Pii logging in the SDK by passing 'true' for piiLogging when using WithLogger() when creating the client. This defaults to false diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index d9da3c34..285ef720 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -202,7 +202,7 @@ func WithRetryPolicyDisabled() ClientOption { // Client to be used to acquire tokens for managed identity. // ID: [SystemAssigned], [UserAssignedClientID], [UserAssignedResourceID], [UserAssignedObjectID] // -// Options: [WithHTTPClient], [WithLogger], [WithPiiLogging] +// Options: [WithHTTPClient], [WithLogger] func New(id ID, options ...ClientOption) (Client, error) { opts := ClientOptions{ httpClient: shared.DefaultClient, diff --git a/apps/managedidentity/managedidentity_with_funcs.go b/apps/managedidentity/managedidentity_with_funcs.go index bd364ea0..269acccb 100644 --- a/apps/managedidentity/managedidentity_with_funcs.go +++ b/apps/managedidentity/managedidentity_with_funcs.go @@ -7,15 +7,10 @@ import ( ) // WithLogger enables logging within the SDK -func WithLogger(l *slog.Logger) ClientOption { +// piiEnabled sets logging of Personally Identifiable Information (PII) within the SDK +func WithLogger(l *slog.Logger, piiEnabled bool) ClientOption { return func(o *ClientOptions) { o.logger = l - } -} - -// WithPiiLogging enables logging of Personally Identifiable Information (PII) within the SDK -func WithPiiLogging() ClientOption { - return func(o *ClientOptions) { - o.piiLogging = true + o.piiLogging = piiEnabled } } diff --git a/apps/managedidentity/managedidentity_with_logger_test.go b/apps/managedidentity/managedidentity_with_logger_test.go index 2004656a..ebda5371 100644 --- a/apps/managedidentity/managedidentity_with_logger_test.go +++ b/apps/managedidentity/managedidentity_with_logger_test.go @@ -42,7 +42,7 @@ func TestClientLogging(t *testing.T) { bufferHandler := &BufferHandler{} customLogger := slog.New(bufferHandler) - client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger)) + client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger, false)) if err != nil { t.Fatal(err) } @@ -65,7 +65,7 @@ func TestClientLogging_CustomHandler(t *testing.T) { filteredBufferHandler := &BufferHandler{level: slog.LevelDebug} customLogger := slog.New(filteredBufferHandler) - client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger)) + client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger, false)) if err != nil { t.Fatal(err) } From 2da41d2764d3084e9d9434b9776f107b31f140ef Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Tue, 4 Feb 2025 22:48:08 +0000 Subject: [PATCH 28/31] * PR Changes --- apps/internal/slog/logger.go | 1 + apps/internal/slog/logger_121_below.go | 5 ++-- apps/internal/slog/logger_test.go | 12 -------- apps/internal/slog/nop_handler.go | 6 ---- apps/managedidentity/managedidentity.go | 7 ++++- .../managedidentity_with_logger_test.go | 29 +++++++------------ ...edidentity_with_funcs.go => options.go.go} | 3 ++ 7 files changed, 23 insertions(+), 40 deletions(-) delete mode 100644 apps/internal/slog/nop_handler.go rename apps/managedidentity/{managedidentity_with_funcs.go => options.go.go} (81%) diff --git a/apps/internal/slog/logger.go b/apps/internal/slog/logger.go index 3ed6376e..2a1ff6a3 100644 --- a/apps/internal/slog/logger.go +++ b/apps/internal/slog/logger.go @@ -21,6 +21,7 @@ const ( type Handler = slog.Handler type Logger = slog.Logger +type NopHandler struct{} func (*NopHandler) Enabled(context.Context, slog.Level) bool { return false } func (*NopHandler) Handle(context.Context, slog.Record) error { return nil } diff --git a/apps/internal/slog/logger_121_below.go b/apps/internal/slog/logger_121_below.go index 18cc1a78..65d74156 100644 --- a/apps/internal/slog/logger_121_below.go +++ b/apps/internal/slog/logger_121_below.go @@ -9,10 +9,11 @@ import ( "context" ) -type Level int - +type NopHandler struct{} type Logger struct{} +type Level int + const ( LevelDebug Level = iota LevelInfo diff --git a/apps/internal/slog/logger_test.go b/apps/internal/slog/logger_test.go index 5920a8a5..80ff5f33 100644 --- a/apps/internal/slog/logger_test.go +++ b/apps/internal/slog/logger_test.go @@ -53,15 +53,3 @@ func TestLogger_Log_ConsoleOutput(t *testing.T) { } } } - -func TestNewLogger_ValidSlogHandler(t *testing.T) { - // Test case where handler is a valid slog.Handler - var buf bytes.Buffer - handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{ - Level: slog.LevelDebug, - }) - logInstance := New(handler) - if logInstance == nil { - t.Fatalf("expected non-nil logInstance, got nil") - } -} diff --git a/apps/internal/slog/nop_handler.go b/apps/internal/slog/nop_handler.go deleted file mode 100644 index 00f6e023..00000000 --- a/apps/internal/slog/nop_handler.go +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -package slog - -type NopHandler struct{} diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index 5ccbf082..04c89070 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -20,6 +20,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "time" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/errors" @@ -132,6 +133,7 @@ var getAzureArcHimdsFilePath = func(platform string) string { return "" } } +var logOnce sync.Once type Source string @@ -271,6 +273,10 @@ func New(id ID, options ...ClientOption) (Client, error) { piiLogging: opts.piiLogging, } + logOnce.Do(func() { + client.logger.Log(context.Background(), slog.LevelInfo, "Managed Identity", slog.String("source", string(client.source))) + }) + fakeAuthInfo, err := authority.NewInfoFromAuthorityURI("https://login.microsoftonline.com/managed_identity", false, true) if err != nil { return Client{}, err @@ -317,7 +323,6 @@ func (c Client) AcquireToken(ctx context.Context, resource string, options ...Ac option(&o) } c.authParams.Scopes = []string{resource} - c.logger.Log(ctx, slog.LevelInfo, "Managed Identity", slog.String("source", string(c.source))) // when claims are empty, we get token from the cache, otherwise acquire a new one if o.claims == "" { diff --git a/apps/managedidentity/managedidentity_with_logger_test.go b/apps/managedidentity/managedidentity_with_logger_test.go index ebda5371..420f6f5d 100644 --- a/apps/managedidentity/managedidentity_with_logger_test.go +++ b/apps/managedidentity/managedidentity_with_logger_test.go @@ -36,9 +36,7 @@ func (h *BufferHandler) WithGroup(name string) slog.Handler { return h } func TestClientLogging(t *testing.T) { mockClient := mock.Client{} - headers := http.Header{} - headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") - mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) + mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized)) bufferHandler := &BufferHandler{} customLogger := slog.New(bufferHandler) @@ -47,34 +45,27 @@ func TestClientLogging(t *testing.T) { t.Fatal(err) } - _, _ = client.AcquireToken(context.Background(), "https://resource") - + client.AcquireToken(context.Background(), "https://resource") logOutput := bufferHandler.buf.String() expectedLogMessage := "Managed Identity" + if !strings.Contains(logOutput, expectedLogMessage) { t.Errorf("expected log message %q not found in output", expectedLogMessage) } -} - -func TestClientLogging_CustomHandler(t *testing.T) { - mockClient := mock.Client{} - headers := http.Header{} - headers.Set("www-authenticate", "Basic realm=/path/to/secret.key") - mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized), mock.WithHTTPHeader(headers)) + mockClient.AppendResponse(mock.WithHTTPStatusCode(http.StatusUnauthorized)) filteredBufferHandler := &BufferHandler{level: slog.LevelDebug} - customLogger := slog.New(filteredBufferHandler) + customLogger = slog.New(filteredBufferHandler) - client, err := New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger, false)) + client, err = New(SystemAssigned(), WithHTTPClient(&mockClient), WithLogger(customLogger, false)) if err != nil { t.Fatal(err) } - _, _ = client.AcquireToken(context.Background(), "https://resource") + client.AcquireToken(context.Background(), "https://resource") + logOutput = filteredBufferHandler.buf.String() - logOutput := filteredBufferHandler.buf.String() - unexpectedLogMessage := "Managed Identity" - if strings.Contains(logOutput, unexpectedLogMessage) { - t.Errorf("unexpected log message %q found in output", unexpectedLogMessage) + if strings.Contains(logOutput, expectedLogMessage) { + t.Errorf("unexpected log message %q found in output", expectedLogMessage) } } diff --git a/apps/managedidentity/managedidentity_with_funcs.go b/apps/managedidentity/options.go.go similarity index 81% rename from apps/managedidentity/managedidentity_with_funcs.go rename to apps/managedidentity/options.go.go index 269acccb..637f528c 100644 --- a/apps/managedidentity/managedidentity_with_funcs.go +++ b/apps/managedidentity/options.go.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + //go:build go1.21 package managedidentity From 72611caf58280f3cdde65facc8476bc5a3689a1e Mon Sep 17 00:00:00 2001 From: Nilesh Choudhary Date: Thu, 6 Feb 2025 13:36:41 +0000 Subject: [PATCH 29/31] Updated the file name --- apps/managedidentity/{options.go.go => options.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename apps/managedidentity/{options.go.go => options.go} (100%) diff --git a/apps/managedidentity/options.go.go b/apps/managedidentity/options.go similarity index 100% rename from apps/managedidentity/options.go.go rename to apps/managedidentity/options.go From 0122e727bd449bda32005b79962875cfacd9eab3 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Wed, 12 Feb 2025 15:38:59 +0000 Subject: [PATCH 30/31] * Address some PR Comments --- apps/managedidentity/managedidentity.go | 8 ++++---- apps/managedidentity/managedidentity_with_logger_test.go | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index 04c89070..f87ae410 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -273,10 +273,6 @@ func New(id ID, options ...ClientOption) (Client, error) { piiLogging: opts.piiLogging, } - logOnce.Do(func() { - client.logger.Log(context.Background(), slog.LevelInfo, "Managed Identity", slog.String("source", string(client.source))) - }) - fakeAuthInfo, err := authority.NewInfoFromAuthorityURI("https://login.microsoftonline.com/managed_identity", false, true) if err != nil { return Client{}, err @@ -317,6 +313,10 @@ func GetSource() (Source, error) { // Resource: scopes application is requesting access to // Options: [WithClaims] func (c Client) AcquireToken(ctx context.Context, resource string, options ...AcquireTokenOption) (base.AuthResult, error) { + logOnce.Do(func() { + c.logger.Log(context.Background(), slog.LevelInfo, "Managed Identity", slog.String("source", string(c.source))) + }) + resource = strings.TrimSuffix(resource, "/.default") o := AcquireTokenOptions{} for _, option := range options { diff --git a/apps/managedidentity/managedidentity_with_logger_test.go b/apps/managedidentity/managedidentity_with_logger_test.go index 420f6f5d..6bf03fac 100644 --- a/apps/managedidentity/managedidentity_with_logger_test.go +++ b/apps/managedidentity/managedidentity_with_logger_test.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + //go:build go1.21 package managedidentity From 6d526339306ab2b0d276b3a6184cf97054b92325 Mon Sep 17 00:00:00 2001 From: Andrew O Hart Date: Thu, 13 Feb 2025 17:15:25 +0000 Subject: [PATCH 31/31] * Use context of the function rather than the logOnce --- apps/managedidentity/managedidentity.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/managedidentity/managedidentity.go b/apps/managedidentity/managedidentity.go index f87ae410..583e6f92 100644 --- a/apps/managedidentity/managedidentity.go +++ b/apps/managedidentity/managedidentity.go @@ -314,7 +314,7 @@ func GetSource() (Source, error) { // Options: [WithClaims] func (c Client) AcquireToken(ctx context.Context, resource string, options ...AcquireTokenOption) (base.AuthResult, error) { logOnce.Do(func() { - c.logger.Log(context.Background(), slog.LevelInfo, "Managed Identity", slog.String("source", string(c.source))) + c.logger.Log(ctx, slog.LevelInfo, "Managed Identity", slog.String("source", string(c.source))) }) resource = strings.TrimSuffix(resource, "/.default")