-
Notifications
You must be signed in to change notification settings - Fork 822
Add azlogs module #1799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Add azlogs module #1799
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Azure Logs module to WTF that enables users to display Azure Log Analytics query results in a table format. The module provides integration with Azure Monitor APIs to query log workspaces using KQL (Kusto Query Language) and displays the results in a formatted table widget.
Key Changes:
- Adds complete Azure Log Analytics integration with authentication and query execution
- Implements table-based data display with adaptive column widths and row truncation
- Provides comprehensive test coverage across all module components
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
modules/azurelogs/widget.go | Core widget implementation with table rendering and data fetching logic |
modules/azurelogs/session.go | Azure authentication and session management |
modules/azurelogs/query.go | Query execution with concurrent client management |
modules/azurelogs/config.go | YAML configuration file parsing for query definitions |
modules/azurelogs/settings.go | Widget settings structure and initialization |
go.mod | Azure SDK dependencies and Go version update |
app/widget_maker.go | Widget registration in the main application |
*_test.go files | Comprehensive test coverage for all components |
Comments suppressed due to low confidence (1)
modules/azurelogs/widget.go:166
- Potential index out of bounds error. The code checks
colIdx < len(colWidths)
on line 166 but then usescolWidths[colIdx]
without verifying the condition is still true. This could cause a panic if colWidths has fewer elements than expected.
if colIdx < len(colWidths) && len(cellText) > colWidths[colIdx] {
modules/azurelogs/widget.go
Outdated
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to" | ||
"strings" | ||
"time" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The import of "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" should be grouped with other third-party imports, not placed between standard library and local imports. Consider moving it after line 9 with other Azure SDK imports.
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to" | |
"strings" | |
"time" | |
"strings" | |
"time" | |
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to" |
Copilot uses AI. Check for mistakes.
case string: | ||
r = append(r, v) | ||
case float64: | ||
r = append(r, fmt.Sprintf("%.0f", v)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using %.0f
format for float64 values will truncate decimal places without rounding. Consider using proper rounding with fmt.Sprintf("%.0f", math.Round(v))
or a more explicit conversion like strconv.FormatFloat(v, 'f', 0, 64)
to make the intention clear.
Copilot uses AI. Check for mistakes.
} | ||
|
||
filename := file.Name() | ||
if len(filename) > 5 && filename[len(filename)-5:] == ".yaml" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file extension check is fragile and doesn't handle .yml files. Consider using strings.HasSuffix(filename, ".yaml") || strings.HasSuffix(filename, ".yml")
or filepath.Ext(filename)
for more robust file extension checking.
if len(filename) > 5 && filename[len(filename)-5:] == ".yaml" { | |
if strings.HasSuffix(filename, ".yaml") || strings.HasSuffix(filename, ".yml") { |
Copilot uses AI. Check for mistakes.
case string: | ||
return "\"" + val + "\"" | ||
case int: | ||
return string(rune(val + '0')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conversion is incorrect for integers greater than 9. string(rune(val + '0'))
only works for single digits (0-9). For larger integers, this will produce incorrect characters. Use strconv.Itoa(val)
instead.
Copilot uses AI. Check for mistakes.
Hi, Some thoughts here.
We should probably walk through the first point before you spend time on 2 and 3. |
In Azure, every service will natively send logs to a service called Azure Log Analytics Workspace. It's the equivalent of an ELK (Elasticsearch, Logstash, Kibana) stack where you, respectively, store, ship, and view/search the logs. Any users of Azure would benefit from this module by being able to see up-to-date results from predefined queries (Kusto is the name of the query language used in Log Analytics Workspace). If you're familiar with AWS, this is the equivalent of AWS CloudWatch. The rate of change is down to the service logging, but it's expected to change every few seconds. How useful this would be to your user base is entirely down to how many would be using Azure for their cloud computing. If that's small/unlikely, then no need to include this and I'll happily continue with it in my private fork. Will rebase asap. |
b113ff9
to
70b32fa
Compare
I need to give this a proper review still but I'm aiming to include this in the next minor release. |
This new module enables table output of logs as queried by a log analytics workspace Kusto query.
Documentation updates with screenshot will follow in the next few days.