allow multi instance sql server

[cwuethrich]

Please label this PR with one of the following labels, depending on the scope of your change:

• Bug

What does this PR do?

This pull requests allow to use metricbeat with MSSQL and named instances.

Why is it important?

Bigger companies often don't use MSSQL in a docker container. Often they have one server with multiple named instances. This change allows them to use metricbeat as well.

Checklist

- [ ] My code follows the style guidelines of this project
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have made corresponding change to the default configuration files
- [ ] I have added tests that prove my fix is effective or that my feature works

• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

• Verify changes SQL statement

How to test this PR locally

Related issues

• Closes metricbeat mssql module: performance counter don't work with named instances #24076

Use cases

Screenshots

Logs

[mergify]

This pull request does not have a backport label. Could you fix it @cwuethrich? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b metricbeat-mssql-instance upstream/metricbeat-mssql-instance
git merge upstream/main
git push upstream metricbeat-mssql-instance

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-04-25T17:44:36.816+0000

• Duration: 32 min 42 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2
Skipped	2
Total	4

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[elasticmachine]

Pinging @elastic/integrations (Team:Integrations)

[rdner]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[rdner]

jenkins run tests

[rdner]

I think we could merge it without fixing the linter issue, @adriansr could you have a look at the code change?

[adriansr]

I'm not comfortable reviewing changes to this large SQL expression. @sayden can you have a look as the original author?

[sayden]

@cwuethrich thanks for contributing this. Let me ask you a couple of important things:

• Is this PR changin the key of any of the output events?
• How this query will behave with beats multi-instance? For me it seems it will generate duplicate data.

Please, also data.json on performance/_meta folder must be regenerated.

Thanks again!

@rameshelastic FYI 🙂

[cwuethrich]

Thank you all for your support.

@sayden :

• Is this PR changin the key of any of the output events?

No, it doesn't change anything. The counter_name is used to identify the events. object_name is different on multi instance SQL Servers. That's why the previous solution didn't work. But this field isn't used. I change the selection to a like-search. A single instance SQL Server has the same result as before.

• How this query will behave with beats multi-instance? For me it seems it will generate duplicate data.

If I understand you right it could happen. But this is exactly the same without this change. It doesn't change anything.

Please, also data.json on performance/_meta folder must be regenerated.

This file doesn't change.

[sayden]

The CI now complains about old code. This is a way to fix some stuff incrementally. I know it's unrelated to your PR but you'll have to fix this too.

We can safely assume that you can change those errors.Wrap(err, "msg") to fmt.Errorf("msg %w", err) and remove the import.

Sorry for this extra step.

Update of _meta/data.json is mandatory in this type of PR, even if only values changes and not keys. You can just copy-paste one example event from the console by running metricbeat with -e -d "*" for example.

Everything else, I don't see any reason to not merge this after those changes.

[cwuethrich]

Everything else, I don't see any reason to not merge this after those changes.

@sayden thank you for your great support. Should be all fixed now.

[sayden]

/test

[sayden]

/test

it seems something got hanged in the previous attempt and I was getting a 500 when triggering an individual step

[sayden]

LGTM Thank you!