Skip to content

[receiver/sqlserver] Add collection interval for top query #40002

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .chloggen/top-query-collection-interval.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: sqlserverreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add configuration option `top_query_collection.collection_interval` for top query collection to make the collection less frequent.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [40002]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
- This change only applies to the `top_query_collection` feature.
- The default value is `60s`
# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
7 changes: 7 additions & 0 deletions receiver/sqlserverreceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ Top-Query collection specific options (only useful when top-query collection are
- Queries that were finished execution outside the lookback window are not included in the collection. Increasing the lookback window (in seconds) will be useful for capturing long-running queries.
- `max_query_sample_count` (optional, example = `5000`, default = `1000`): The maximum number of records to fetch in a single run.
- `top_query_count`: (optional, example = `100`, default = `200`): The maximum number of active queries to report (to the next consumer) in a single run.
- `collection_interval`: (optional, default = `60s`): The interval at which top queries should be emitted by this receiver.
- This value can only guarantee that the top queries are collected at most once in this interval.
- For instance, you have global `collection_interval` as `10s` and `top_query_collection.collection_interval` as `60s`.
- In this case, the default receiver scraper will still try to run in every 10 seconds.
- However, the top queries collection will only run after 60 seconds have passed since the last collection.
- For instance, you have global `collection_interval` as `10s` and `top_query_collection.collection_interval` as `5s`.
- In this case, `top_query_collection.collection_internal` will make no effects to the collection
Comment on lines +51 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it should be moved out from the attribute definitions, and into its own section that can explain it in more detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This document is specifically intended for configurations, so it seems appropriate to include it here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a user who is new to the component, this is a lot of context provided as dot points. To help give a deeper background and context, it should be moved to its own section and expanded upon in a paragraph so examples can also be given.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't call this a blocking action, but as a user it would be nice to provide that extra context in a more naturally written form.

- `enabled`: (optional, default = `false`): Enable collection of top queries.
- e.g. `sqlserver` receiver will fetch 1000 (value: `max_query_sample_count`) queries from database and report the top 200 (value: `top_query_count`) which used the most CPU time.

Expand Down
14 changes: 10 additions & 4 deletions receiver/sqlserverreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package sqlserverreceiver // import "github.com/open-telemetry/opentelemetry-col

import (
"errors"
"time"

"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/scraper/scraperhelper"
Expand All @@ -23,10 +24,11 @@ type TopQueryCollection struct {
// The query statement will also be reported, hence, it is not ideal to send it as a metric. Hence
// we are reporting them as logs.
// The `N` is configured via `TopQueryCount`
Enabled bool `mapstructure:"enabled"`
LookbackTime uint `mapstructure:"lookback_time"`
MaxQuerySampleCount uint `mapstructure:"max_query_sample_count"`
TopQueryCount uint `mapstructure:"top_query_count"`
Enabled bool `mapstructure:"enabled"`
LookbackTime uint `mapstructure:"lookback_time"`
MaxQuerySampleCount uint `mapstructure:"max_query_sample_count"`
TopQueryCount uint `mapstructure:"top_query_count"`
CollectionInterval time.Duration `mapstructure:"collection_interval"`
}

// Config defines configuration for a sqlserver receiver.
Expand Down Expand Up @@ -72,6 +74,10 @@ func (cfg *Config) Validate() error {
return errors.New("`top_query_count` must be less than or equal to `max_query_sample_count`")
}

if cfg.TopQueryCollection.CollectionInterval < 0 {
return errors.New("`top_query_collection.collection_interval` must not be less than 0")
}

cfg.isDirectDBConnectionEnabled, err = directDBConnectionEnabled(cfg)

return err
Expand Down
1 change: 1 addition & 0 deletions receiver/sqlserverreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func createDefaultConfig() component.Config {
LookbackTime: uint(2 * cfg.CollectionInterval / time.Second),
MaxQuerySampleCount: 1000,
TopQueryCount: 200,
CollectionInterval: time.Minute,
},
}
}
Expand Down
1 change: 1 addition & 0 deletions receiver/sqlserverreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func TestFactory(t *testing.T) {
LookbackTime: uint(2 * 10),
MaxQuerySampleCount: 1000,
TopQueryCount: 200,
CollectionInterval: time.Minute,
},
QuerySample: QuerySample{
Enabled: false,
Expand Down
56 changes: 32 additions & 24 deletions receiver/sqlserverreceiver/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,20 @@ const (
)

type sqlServerScraperHelper struct {
id component.ID
config *Config
sqlQuery string
instanceName string
clientProviderFunc sqlquery.ClientProviderFunc
dbProviderFunc sqlquery.DbProviderFunc
logger *zap.Logger
telemetry sqlquery.TelemetryConfig
client sqlquery.DbClient
db *sql.DB
mb *metadata.MetricsBuilder
lb *metadata.LogsBuilder
cache *lru.Cache[string, int64]
id component.ID
config *Config
sqlQuery string
instanceName string
clientProviderFunc sqlquery.ClientProviderFunc
dbProviderFunc sqlquery.DbProviderFunc
logger *zap.Logger
telemetry sqlquery.TelemetryConfig
client sqlquery.DbClient
db *sql.DB
mb *metadata.MetricsBuilder
lb *metadata.LogsBuilder
cache *lru.Cache[string, int64]
lastExecutionTimestamp time.Time
}

var (
Expand All @@ -67,16 +68,17 @@ func newSQLServerScraper(id component.ID,
cache *lru.Cache[string, int64],
) *sqlServerScraperHelper {
return &sqlServerScraperHelper{
id: id,
config: cfg,
sqlQuery: query,
logger: params.Logger,
telemetry: telemetry,
dbProviderFunc: dbProviderFunc,
clientProviderFunc: clientProviderFunc,
mb: metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, params),
lb: metadata.NewLogsBuilder(cfg.LogsBuilderConfig, params),
cache: cache,
id: id,
config: cfg,
sqlQuery: query,
logger: params.Logger,
telemetry: telemetry,
dbProviderFunc: dbProviderFunc,
clientProviderFunc: clientProviderFunc,
mb: metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, params),
lb: metadata.NewLogsBuilder(cfg.LogsBuilderConfig, params),
cache: cache,
lastExecutionTimestamp: time.Unix(0, 0),
}
}

Expand Down Expand Up @@ -121,6 +123,10 @@ func (s *sqlServerScraperHelper) ScrapeLogs(ctx context.Context) (plog.Logs, err
var resources pcommon.Resource
switch s.sqlQuery {
case getSQLServerQueryTextAndPlanQuery():
if s.lastExecutionTimestamp.Add(s.config.TopQueryCollection.CollectionInterval).After(time.Now()) {
s.logger.Debug("Skipping the collection of top queries because the current time has not yet exceeded the last execution time plus the specified collection interval")
return plog.NewLogs(), nil
}
resources, err = s.recordDatabaseQueryTextAndPlan(ctx, s.config.TopQueryCount)
case getSQLServerQuerySamplesQuery():
resources, err = s.recordDatabaseSampleQuery(ctx)
Expand Down Expand Up @@ -596,7 +602,9 @@ func (s *sqlServerScraperHelper) recordDatabaseQueryTextAndPlan(ctx context.Cont
sort.Slice(totalElapsedTimeDiffsMicrosecond, func(i, j int) bool { return totalElapsedTimeDiffsMicrosecond[i] > totalElapsedTimeDiffsMicrosecond[j] })

resourcesAdded := false
timestamp := pcommon.NewTimestampFromTime(time.Now())
now := time.Now()
timestamp := pcommon.NewTimestampFromTime(now)
s.lastExecutionTimestamp = now
for i, row := range rows {
// skipping the rest of the rows as totalElapsedTimeDiffs is sorted in descending order
if totalElapsedTimeDiffsMicrosecond[i] == 0 {
Expand Down
1 change: 1 addition & 0 deletions receiver/sqlserverreceiver/scraper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ func TestQueryTextAndPlanQuery(t *testing.T) {

configureAllScraperMetrics(cfg, false)
cfg.TopQueryCollection.Enabled = true
cfg.TopQueryCollection.CollectionInterval = cfg.ControllerConfig.CollectionInterval

scrapers := setupSQLServerLogsScrapers(receivertest.NewNopSettings(metadata.Type), cfg)
assert.NotNil(t, scrapers)
Expand Down
Loading