Skip to content

[e2e] Add tests for self monitoring agent logs in otel mode#6801

Merged
khushijain21 merged 76 commits intoelastic:mainfrom
khushijain21:monitoringdocs
Mar 20, 2025
Merged

[e2e] Add tests for self monitoring agent logs in otel mode#6801
khushijain21 merged 76 commits intoelastic:mainfrom
khushijain21:monitoringdocs

Conversation

@khushijain21
Copy link
Contributor

@khushijain21 khushijain21 commented Feb 11, 2025

What does this PR do?

It adds tests for comparing docs ingested by agent.monitoring vs otel.yml

Notes:
Some changes made to otel.yml produced by hand here:

Checklist

  • I have read and understood the pull request guidelines of this project.
  • 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/fragments using the changelog tool
  • I have added an integration test or an E2E test

How to test this PR locally

Related issues

@khushijain21 khushijain21 requested a review from a team as a code owner February 11, 2025 07:54
@khushijain21 khushijain21 marked this pull request as draft February 11, 2025 07:54
@mergify
Copy link
Contributor

mergify bot commented Feb 11, 2025

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

@mergify
Copy link
Contributor

mergify bot commented Feb 11, 2025

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

  • backport-./d./d is the label that automatically backports to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@khushijain21 khushijain21 force-pushed the monitoringdocs branch 3 times, most recently from c45b743 to cdda948 Compare February 14, 2025 06:28
@pierrehilbert pierrehilbert added skip-changelog Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Feb 14, 2025
@khushijain21
Copy link
Contributor Author

khushijain21 commented Mar 17, 2025

The check-ci fails because integration tests here are running inside t.Run() blocks for readability and keeping sub tests isolated. Can we redefine this check to accommodate this?

The failure -

Running dependency: main.Integration.Check
>> check: Checking for define.Require in integration tests
Error: test TestAgentMonitoring first statement must be a function call to define.Require
make: *** [Makefile:29: check-ci] Error 1

cc: @elastic/ingest-eng-prod

@pchila
Copy link
Member

pchila commented Mar 17, 2025

The check-ci fails because integration tests here are running inside t.Run() blocks for readability and keeping sub tests isolated. Can we redefine this check to accommodate this?

The failure -

Running dependency: main.Integration.Check
>> check: Checking for define.Require in integration tests
Error: test TestAgentMonitoring first statement must be a function call to define.Require
make: *** [Makefile:29: check-ci] Error 1

The check for define.Require() at the beginning of the outer test function has been introduced because the initial integration test framework rely on it to skip the whole test during the discovery phase while dumping the requirements without any extra logs.
See

func defineAction(t *testing.T, req Requirements) *Info {

and
func DetermineBatches(dir string, testFlags string, buildTags ...string) ([]Batch, error) {

The check itself is defined here

func (Integration) Check() error {

and it parses the integration tests directory for *_test.go files containing Test* functions and validating that the first statement in the ast is a define.Require call

Your PR is not passing this check since the define.Require call has been wrapped into a t.Run() in TestAgentMonitoring, just move it out of t.Run and the check will pass. This is a limitation of the initial integration framework implementation that does not have full support for subtests with different requirements. Since you have different requirements across the subtests (like here and here ) you may need to split the TestAgentMonitoring test function into multiple ones and extract the define.Require at the Test* function level.

Maybe @blakerouse as the original implementer of the check and the batching may have another solution.

@pchila
Copy link
Member

pchila commented Mar 17, 2025

Add-on to my previous comment: after chatting with @khushijain21 it seems that the only difference between the 2 sub-tests is that this subtest must not be run on windows.

The same thing can be achieved by using a single define.Require() call as it was defined before this change and add a t.Skip() on the second sub-test if the OS is windows. It's not very elegant but it should achieve what you need without triggering the integration test mage check.

@khushijain21 khushijain21 requested a review from leehinman March 17, 2025 15:11
@cla-checker-service
Copy link

cla-checker-service bot commented Mar 18, 2025

💚 CLA has been signed

@elasticmachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

cc @khushijain21

@elastic-sonarqube
Copy link

@khushijain21 khushijain21 requested a review from leehinman March 18, 2025 11:55
@khushijain21 khushijain21 enabled auto-merge (squash) March 20, 2025 04:13
@khushijain21 khushijain21 merged commit 931134f into elastic:main Mar 20, 2025
12 checks passed
mergify bot pushed a commit that referenced this pull request Mar 20, 2025
- Add tests for self monitoring agent logs in otel mode

(cherry picked from commit 931134f)

# Conflicts:
#	testing/integration/beat_receivers_test.go
khushijain21 added a commit that referenced this pull request Mar 24, 2025
…7479)

- Add tests for self monitoring agent logs in otel mode

(cherry picked from commit 931134f)

# Conflicts:
#	testing/integration/beat_receivers_test.go

Co-authored-by: Khushi Jain <khushi.jain@elastic.co>
@khushijain21 khushijain21 added the backport-9.0 Automated backport to the 9.0 branch label Apr 24, 2025
mergify bot pushed a commit that referenced this pull request Apr 24, 2025
- Add tests for self monitoring agent logs in otel mode

(cherry picked from commit 931134f)

# Conflicts:
#	testing/integration/beat_receivers_test.go
khushijain21 added a commit that referenced this pull request Apr 24, 2025
- Add tests for self monitoring agent logs in otel mode
khushijain21 pushed a commit that referenced this pull request Apr 28, 2025
…in otel mode (#7950)

* [e2e] Add tests for self monitoring agent logs in otel mode (#6801)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-8.x Automated backport to the 8.x branch with mergify backport-9.0 Automated backport to the 9.0 branch skip-changelog Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.