-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Generic SQL join results of multiple queries #32394
Changes from all commits
a201f83
d9cc266
ec73740
66b810d
bd816b0
211e617
9d08c54
7725de3
4c7b310
dc5ca3a
212f651
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,8 @@ type config struct { | |
ResponseFormat string `config:"sql_response_format"` | ||
Query string `config:"sql_query" ` | ||
|
||
Queries []query `config:"sql_queries" ` | ||
Queries []query `config:"sql_queries" ` | ||
MergeResults bool `config:"merge_results"` | ||
} | ||
|
||
// MetricSet holds any configuration or state information. It must implement | ||
|
@@ -124,6 +125,8 @@ func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error { | |
queries = append(queries, one_query) | ||
} | ||
|
||
merged := mapstr.M{} | ||
|
||
for _, q := range queries { | ||
if q.ResponseFormat == tableResponseFormat { | ||
// Table format | ||
|
@@ -133,7 +136,22 @@ func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error { | |
} | ||
|
||
for _, ms := range mss { | ||
m.reportEvent(ms, reporter, q.Query) | ||
if m.Config.MergeResults { | ||
if len(mss) > 1 { | ||
return fmt.Errorf("can not merge query resulting with more than one rows: %s", q) | ||
} else { | ||
for k, v := range ms { | ||
_, ok := merged[k] | ||
if ok { | ||
m.Logger().Warn("overwriting duplicate metrics: ", k) | ||
} | ||
merged[k] = v | ||
} | ||
} | ||
} else { | ||
// Report immediately for non-merged cases. | ||
m.reportEvent(ms, reporter, q.Query) | ||
} | ||
} | ||
} else { | ||
// Variable format | ||
|
@@ -142,9 +160,24 @@ func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error { | |
return fmt.Errorf("fetch variable mode failed: %w", err) | ||
} | ||
|
||
m.reportEvent(ms, reporter, q.Query) | ||
if m.Config.MergeResults { | ||
for k, v := range ms { | ||
_, ok := merged[k] | ||
if ok { | ||
m.Logger().Warn("overwriting duplicate metrics: ", k) | ||
} | ||
merged[k] = v | ||
} | ||
} else { | ||
// Report immediately for non-merged cases. | ||
m.reportEvent(ms, reporter, q.Query) | ||
} | ||
} | ||
} | ||
if m.Config.MergeResults { | ||
// Report here for merged case. | ||
m.reportEvent(merged, reporter, "") | ||
} | ||
|
||
return nil | ||
} | ||
|
@@ -154,20 +187,32 @@ func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error { | |
func (m *MetricSet) reportEvent(ms mapstr.M, reporter mb.ReporterV2, qry string) { | ||
if m.Config.RawData.Enabled { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another way to do this would be:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I hope we can stay with the current structure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, you can do it this way in case you feel it complicates less the code 🙂 #32394 (comment) |
||
reporter.Event(mb.Event{ | ||
// New usage. | ||
// Only driver & query field mapped. | ||
// metrics to be mapped by end user. | ||
ModuleFields: mapstr.M{ | ||
"metrics": ms, // Individual metric | ||
"driver": m.Config.Driver, | ||
"query": qry, | ||
}, | ||
}) | ||
// New usage. | ||
// Only driver & query field mapped. | ||
// metrics to be mapped by end user. | ||
if len(qry) > 0 { | ||
// set query. | ||
reporter.Event(mb.Event{ | ||
ModuleFields: mapstr.M{ | ||
"metrics": ms, // Individual metric | ||
"driver": m.Config.Driver, | ||
"query": qry, | ||
}, | ||
}) | ||
} else { | ||
reporter.Event(mb.Event{ | ||
// Do not set query. | ||
ModuleFields: mapstr.M{ | ||
"metrics": ms, // Individual metric | ||
"driver": m.Config.Driver, | ||
}, | ||
}) | ||
|
||
} | ||
} else { | ||
// Previous usage. Backword compartibility. | ||
// Supports field mapping. | ||
reporter.Event(mb.Event{ | ||
// Previous usage. Backword compartibility. | ||
// Supports field mapping. | ||
ModuleFields: mapstr.M{ | ||
"driver": m.Config.Driver, | ||
"query": qry, | ||
|
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.
Nit. This is getting more complicated now, maybe the code for variable and table format could be moved to separated functions.