Skip to content

Conversation

@jsoriano
Copy link
Member

Migrate stack monitoring packages to spec 3.x.

@jsoriano jsoriano self-assigned this Dec 18, 2024
@andrewkroh andrewkroh added dashboard Relates to a Kibana dashboard bug, enhancement, or modification. Integration:beat Beat Integration:elasticsearch Elasticsearch Integration:kibana Kibana labels Dec 18, 2024
@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Dec 18, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@consulthys
Copy link
Contributor

@jsoriano Thanks for initiating this. Quick noob question: why did you use 3.0.4 instead of 3.3.1 as you commented here?

@jsoriano
Copy link
Member Author

why did you use 3.0.4 instead of 3.3.1 as you commented here?

This package is intended for versions of kibana over 8.10.2. Kibana till 8.16 was filtering out packages up to 3.0 versions.
In summary, for packages supporting versions of kibana below 8.16, it is safer to use 3.0.

@jsoriano jsoriano mentioned this pull request Dec 19, 2024
4 tasks
@consulthys
Copy link
Contributor

Thanks @jsoriano

Kibana till 8.16 was filtering out packages up to 3.0 versions.

Where can I learn about such Kibana-specific constraints?

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for switching to flattened is because otherwise object_type would be required?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, type: object alone is not allowed because it is like an empty mapping.

I could not find an example document with this field, so I have chosen flattened as a quite safe option. But it would be indeed better to provide a more specific mapping. Do you know what is the structure of these fields?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can actually find a sample event containing some data for that field here, which only contains two subfields, one keyword and one text, which I guess rules out the object_type option altogether:

 "read_exceptions": { "type": "nested", "properties": { "exception": { "type": "object", "properties": { "reason": { "type": "text" }, "type": { "type": "keyword" } } }, 

Also interesting is to see how that field is actually used in Stack Monitoring, i.e. the query only checks for the existence of the exceptions object (which flattened also supports), and then the type subfield is read.

I'll do some more checks to see if changing from object to flattened doesn't break anything (also in the case of monitoring indices not having the same mapping after rollover)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, these mappings with nested sound good too, I can try with them.

@consulthys
Copy link
Contributor

As a result of seeing the output of elastic-package build, all the changes make sense (except the flattened one I flagged)

@jsoriano
Copy link
Member Author

Kibana till 8.16 was filtering out packages up to 3.0 versions.

Where can I learn about such Kibana-specific constraints?

At the moment this is only in the default kibana configs. We should probably document this somewhere else, similar to what we have for kibana constraints in https://github.com/elastic/elastic-package/blob/main/docs/howto/stack_version_support.md#when-to-update-the-condition.

@jsoriano
Copy link
Member Author

Adding guidelines for spec version to elastic/elastic-package#2291

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the first safe version to use secrets. We can also delay the adoption of secrets, then I would revert the change in the kibana constraint, and set secret: false in the cases where it is set to true now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be fine bumping from 8.10.1 to 8.11.2

@jsoriano jsoriano marked this pull request as ready for review December 19, 2024 18:22
@jsoriano jsoriano requested a review from a team as a code owner December 19, 2024 18:22
@andrewkroh andrewkroh added the Team:Stack Monitoring Stack Monitoring team [elastic/stack-monitoring] label Dec 19, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be just read_exceptions since exception is now a field of its own below?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, good catch, thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I hadn't noticed that there was already a definition from these fields. I think we only need to remove the definition of the read_exceptions.exception object.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should also probably be 8.11.2

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't updated the version here because this package doesn't make use of secrets, or afaik anything not available in 8.10.

@elastic-sonarqube
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
64.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@botelastic
Copy link

botelastic bot commented Jan 22, 2025

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added Stalled and removed Stalled labels Jan 22, 2025
@elasticmachine
Copy link

elasticmachine commented Jan 28, 2025

💔 Build Failed

Failed CI Steps

History

  • 💚 Build #19769 succeeded 3171757a25430633ffcd810396cbf9a41ff11308
  • 💚 Build #19726 succeeded eea78177aae5d0256543fd64a5e7e5ee34bacffa
  • 💔 Build #19677 failed 06ef9986c184c0414e2801e365270a22a9885224

cc @jsoriano

@elastic-sonarqube
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
60.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@jsoriano
Copy link
Member Author

Elasticsearch migration to spec 3.0 is blocked by elastic/package-spec#868 after the recent addition of transforms in the package.

@qcorporation qcorporation requested review from a team as code owners February 4, 2025 03:56
@andrewkroh andrewkroh added Integration:1password 1Password (Partner supported) Integration:abnormal_security Abnormal AI New Integration Issue or pull request for creating a new integration package. Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Feb 4, 2025
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@qcorporation qcorporation force-pushed the main branch 2 times, most recently from eda4138 to f728ca7 Compare February 5, 2025 22:00
@jsoriano jsoriano closed this Feb 6, 2025
@jsoriano jsoriano deleted the update-stack-monitoring-spec-3.x branch February 6, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dashboard Relates to a Kibana dashboard bug, enhancement, or modification. Integration:abnormal_security Abnormal AI Integration:beat Beat Integration:elasticsearch Elasticsearch Integration:kibana Kibana Integration:1password 1Password (Partner supported) New Integration Issue or pull request for creating a new integration package. Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] Team:Stack Monitoring Stack Monitoring team [elastic/stack-monitoring]

4 participants