[Beats] Adding support for elastic-agent-autodiscovery v0.6.13 Introduce log message for not supported annotations #38213

gizas · 2024-03-07T13:03:53Z

Note: Dont merge until elastic/elastic-agent-autodiscover#81 is merged.
We need to udpate go.mod of the beats with v0.6.9 PR is merged

Enhancement

Proposed commit message

Relates with the updates of shared elastic/elastic-agent-autodiscover#81.
In more details we updated utils.GenerateHints to be able to check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic

WHAT: Support of elastic-agent-autodiscovery version 0.6.9 in beats
WHY: the rationale/motivation for the changes

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.

How to test this PR locally

Checkout this branch
Update hints.go file of your /go/pkg/mod/github.com/elastic/elastic-agent-autodiscover@v0.6.8/utils as per hints.go
Build metricbeat/ filebeat locally (as per doc)
Configure autodiscovery in your manifest
Configure some "wrong" co.elastic.hints annotation inside a pod.
Warning messages should be available.

Related issues

Relates #Introduce checks for not supported annotations for Hints based autodiscover of Elastic Agent elastic-agent-autodiscover#81

Logs

mergify · 2024-03-07T13:04:31Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @gizas? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2024-03-07T15:33:45Z

💚 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: 2024-04-25T13:53:54.847+0000
Duration: 241 min 42 sec

Test stats 🧪

Test	Results
Failed	0
Passed	28811
Skipped	1911
Total	30722

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the 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!)

tetianakravchenko · 2024-03-13T15:49:50Z

libbeat/autodiscover/providers/kubernetes/config.go

@@ -30,6 +30,8 @@ import (
 	"github.com/elastic/elastic-agent-libs/logp"
 )

+var AllSupportedHints = []string{"enabled", "module", "metricsets", "host", "period", "timeout", "metricspath", "username", "password", "stream", "processors", "multiline", "json", "disable", "ssl", "metrics_filters", "raw", "include_lines", "exclude_lines", "fileset", "pipeline"}


does metricspath actually works? shouldn't it be metrics_path based on https://www.elastic.co/guide/en/beats/metricbeat/current/configuration-autodiscover-hints.html#_co_elastic_metricsmetrics_path ?

where disable is coming from?

Also as I understand it is a merged list for the metricbeat and for the filebeat, correct? can you please add a comment for that? Is there any way to split it?

I changed metricspath to metrics_path ! Nice catch, thanks Tania!

Disable comes from this: https://github.com/elastic/elastic-agent-autodiscover/blob/7490558445cb8d3248462255edc8ac64bc18b8eb/utils/hints_test.go#L177
We can disable logs collection apparently

I have added a comment in 7d2c429

Currently we parse annotations based on the fact that start with the prefix (L209) and then we split with "/" symbol (L215)
So currently this is a unified solution for elastic-agent and beats. I deliberately had moved the solution to the elastic-agent-autodiscovery in oder not to introduce an additional annotation loop and different implementations. So splitting it for beats will require additional checks if we have Prefix.logs or Prefix.metrics. I consider it out of scope here, mainly because the main reason behind those changes, is to offer the feature in elastic-agent and not in beats. Hope it makes sense

tetianakravchenko · 2024-03-13T15:57:31Z

libbeat/autodiscover/providers/docker/docker.go

@@ -383,7 +383,7 @@ func (d *Provider) generateHints(event bus.Event) bus.Event {
 		e["ports"] = ports
 	}
 	if labels, err := dockerMeta.GetValue("labels"); err == nil {
-		hints := utils.GenerateHints(labels.(mapstr.M), "", d.config.Prefix)
+		hints, _ := utils.GenerateHints(labels.(mapstr.M), "", d.config.Prefix, []string{})


didn't know we call GenerateHints here. Looking on this use case, wondering if it wouldn't be a better option to introduce additionally smth like GenerateHintsWithValidation instead - maybe we should consider it in future for cases when signature is changed

Here GenerateHints will actually return all hints as incorrect but we just ignore them. I think we should make it more clear. Maybe add a bool (validate), which is then checked in GenerateHints

FYI: elastic/elastic-agent-autodiscover#89

rdner

There is still one more linter issue to be resolved but the rest looks good to me.

gizas · 2024-04-24T09:01:08Z

@rdner I added the skip based on comment becaus this is a test f3b6f21#diff-9f24ba274864b933a726842c2b9fb8ee487a13545579938f9e373d0463611142R2200

gizas · 2024-04-24T09:10:37Z

@rdner have you seen this https://github.com/elastic/beats/actions/runs/8813847492/job/24193021847?pr=38213 again?

gizas · 2024-04-24T11:07:08Z

@elastic/beats-tech-leads can I have a review here as well?

mergify · 2024-04-24T14:18:08Z

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 logshints upstream/logshints
git merge upstream/main
git push upstream logshints

rdner · 2024-04-24T16:38:43Z

@rdner have you seen this https://github.com/elastic/beats/actions/runs/8813847492/job/24193021847?pr=38213 again?

@gizas Looks like something is wrong with the Ubuntu itself, it failed on buildkite too. I think we should try to restart, I'm going to update the branch.

gizas · 2024-04-25T09:04:11Z

@rdner this looks better now. I tried to upddate the go.mod and align it as well. The failing tests (mainly related to Windows) I think are not related with changes here

lalit-satapathy

Codeowner approval.

adding support for logshints and autodiscovery 0.6.9

6dba856

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 7, 2024

gizas mentioned this pull request Mar 7, 2024

Introduce log message for not supported annotations for Hints based autodiscover of Elastic Agent elastic/elastic-agent#3064

Closed

mergify bot assigned gizas Mar 7, 2024

gizas added the Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team label Mar 7, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 7, 2024

gizas changed the title ~~adding support for logshints and autodiscovery 0.6.9~~ [Beats] Adding support for elastic-agent-autodiscovery 0.6.9. Introduce log message for not supported annotations Mar 7, 2024

gizas added 3 commits March 8, 2024 11:37

fixing test errors

39b32e9

fixing test errors

8c08dff

adding changelog

51728af

gizas marked this pull request as ready for review March 8, 2024 10:20

gizas requested review from a team as code owners March 8, 2024 10:20

gizas requested review from gsantoro, MichaelKatsoulis, belimawr and rdner March 8, 2024 10:20

gizas added 5 commits March 8, 2024 12:20

Merge branch 'main' into logshints

4009d15

updating go.mod with v0.6.9 autodiscovery library

6985610

Merge branch 'main' into logshints

e023e50

Merge branch 'logshints' of github.com:elastic/beats into logshints

4661676

updating Notice.txt

3e1e1fb

gizas mentioned this pull request Mar 13, 2024

[Elastic Agent]-PR- Introduce log message for not supported annotations for Hints based autodiscover elastic/elastic-agent#4360

Merged

7 tasks

Merge branch 'main' into logshints

edb7c3a

tetianakravchenko reviewed Mar 13, 2024

View reviewed changes

gizas added 2 commits March 13, 2024 18:36

Merge branch 'main' into logshints

3542df5

updating AllSupportedHints list

7d2c429

gizas added 4 commits April 24, 2024 10:09

adding support for docker

92c266b

Merge branch 'logshints' of github.com:elastic/beats into logshints

05d71ef

Merge branch 'main' into logshints

95f30aa

fixing linting error

ea50b00

rdner approved these changes Apr 24, 2024

View reviewed changes

adding comment to skip validation error of test

f3b6f21

Merge branch 'main' into logshints

58da62a

gizas added 3 commits April 24, 2024 17:50

fixing conflicts

e3ca45d

Merge branch 'logshints' of github.com:elastic/beats into logshints

6e282d0

fixing conflicts

e0c255d

rdner and others added 6 commits April 24, 2024 18:38

Merge branch 'main' into logshints

8ae7814

Merge branch 'main' into logshints

58f476f

fixing conflicts

63131b2

fixing conflicts with go.sum

14815b2

fixing conflicts with go.sum

7954069

fixing conflicts with go.sum

7bfd450

tetianakravchenko approved these changes Apr 25, 2024

View reviewed changes

lalit-satapathy approved these changes Apr 25, 2024

View reviewed changes

Merge branch 'main' into logshints

7b5a9bb

gizas enabled auto-merge (squash) April 25, 2024 14:00

gizas disabled auto-merge April 25, 2024 14:21

gizas enabled auto-merge (squash) April 25, 2024 14:22

gizas merged commit 5d1b442 into main Apr 25, 2024
218 checks passed

gizas deleted the logshints branch April 25, 2024 17:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Beats] Adding support for elastic-agent-autodiscovery v0.6.13 Introduce log message for not supported annotations #38213

[Beats] Adding support for elastic-agent-autodiscovery v0.6.13 Introduce log message for not supported annotations #38213

gizas commented Mar 7, 2024 •

edited

mergify bot commented Mar 7, 2024

elasticmachine commented Mar 7, 2024 •

edited

Build stats

Test stats 🧪

tetianakravchenko Mar 13, 2024

gizas Mar 13, 2024 •

edited

tetianakravchenko Mar 13, 2024

MichaelKatsoulis Apr 16, 2024

gizas Apr 16, 2024

rdner left a comment

gizas commented Apr 24, 2024

gizas commented Apr 24, 2024

gizas commented Apr 24, 2024

mergify bot commented Apr 24, 2024

rdner commented Apr 24, 2024

gizas commented Apr 25, 2024

lalit-satapathy left a comment

[Beats] Adding support for elastic-agent-autodiscovery v0.6.13 Introduce log message for not supported annotations #38213

[Beats] Adding support for elastic-agent-autodiscovery v0.6.13 Introduce log message for not supported annotations #38213

Conversation

gizas commented Mar 7, 2024 • edited

Proposed commit message

Checklist

How to test this PR locally

Related issues

Logs

mergify bot commented Mar 7, 2024

elasticmachine commented Mar 7, 2024 • edited

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

tetianakravchenko Mar 13, 2024

Choose a reason for hiding this comment

gizas Mar 13, 2024 • edited

Choose a reason for hiding this comment

tetianakravchenko Mar 13, 2024

Choose a reason for hiding this comment

MichaelKatsoulis Apr 16, 2024

Choose a reason for hiding this comment

gizas Apr 16, 2024

Choose a reason for hiding this comment

rdner left a comment

Choose a reason for hiding this comment

gizas commented Apr 24, 2024

gizas commented Apr 24, 2024

gizas commented Apr 24, 2024

mergify bot commented Apr 24, 2024

rdner commented Apr 24, 2024

gizas commented Apr 25, 2024

lalit-satapathy left a comment

Choose a reason for hiding this comment

gizas commented Mar 7, 2024 •

edited

elasticmachine commented Mar 7, 2024 •

edited

gizas Mar 13, 2024 •

edited