Skip to content

Make xpack.otel_data.registry.enabled default to true #113468

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
merged 6 commits into from
Oct 1, 2024

Conversation

gregkalapos
Copy link
Contributor

Solves #113368

Enabling otel-data by default.

Testing

I removed setting("xpack.otel_data.registry.enabled", "true") from tests, so naturally all test will assume default config and test that the plugin is turned on.

@gregkalapos gregkalapos self-assigned this Sep 24, 2024
@elasticsearchmachine elasticsearchmachine added external-contributor Pull request authored by a developer outside the Elasticsearch team v9.0.0 labels Sep 24, 2024
@gregkalapos gregkalapos removed the external-contributor Pull request authored by a developer outside the Elasticsearch team label Sep 24, 2024
@gregkalapos
Copy link
Contributor Author

@elasticmachine update branch

@gregkalapos gregkalapos marked this pull request as ready for review September 26, 2024 07:13
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Sep 26, 2024
@gregkalapos gregkalapos requested review from a team and felixbarny September 26, 2024 07:15
@gregkalapos gregkalapos added the Team:Data Management Meta label for data/management team label Sep 26, 2024
@elasticsearchmachine elasticsearchmachine removed the Team:Data Management Meta label for data/management team label Sep 26, 2024
@gregkalapos gregkalapos added the Team:Data Management Meta label for data/management team label Sep 27, 2024
@elasticsearchmachine elasticsearchmachine removed the Team:Data Management Meta label for data/management team label Sep 27, 2024
@gregkalapos
Copy link
Contributor Author

@elasticmachine update branch

@gregkalapos
Copy link
Contributor Author

This fails with:

---
No features should be used just by starting up with default configuration:
- do:
    license.get_feature_usage: {}
    headers:
      Content-Type: "application/vnd.elasticsearch+json;compatible-with=8"
      Accept: "application/vnd.elasticsearch+json;compatible-with=8"
- length:
    features: 0

[2024-09-27T21:43:42,054][INFO ][o.e.l.XPackCoreClientYamlTestSuiteIT] [test] Stash dump on test failure [{
  "stash" : {
    "request_headers" : {
      "Accept" : "application/vnd.elasticsearch+json;compatible-with=8",
      "Content-Type" : "application/vnd.elasticsearch+json;compatible-with=8"
    },
    "body" : {
      "features" : [
        {
          "family" : "mappings",
          "name" : "synthetic-source",
          "context" : null,
          "last_used" : "2024-09-27T16:43:40.454Z",
          "license_level" : "enterprise"
        }
      ]
    }
  }
}]

That makes me think, synthetic-source requires some license, but since we use that by default now (otel-data would be on by default and that would need synthetic source), this test fails.

We set _source to synthetic in traces-otel@mappings - strangely when I remove that, this test still fails.

@gregkalapos gregkalapos added the Team:Data Management Meta label for data/management team label Sep 27, 2024
@elasticsearchmachine elasticsearchmachine removed the Team:Data Management Meta label for data/management team label Sep 27, 2024
@felixbarny
Copy link
Member

This isn't really about the usage of synthetic _source itself but is related to this fallback check for LogsDB

/**
* @return whether synthetic source mode should fallback to stored source.
*/
public boolean fallbackToStoredSource() {
if (syntheticSourceFallback) {
return true;
}
return SYNTHETIC_SOURCE_FEATURE.check(licenseState) == false;
}

By calling check, the synthetic-source feature usage gets tracked.
@martijnvg I wonder if it makes sense do adjust the way the fallback is implemented to not consider executing the fallback check usage of synthetic _source. I'd argue that actual usage only happens when an index is created with synthetic _source being used (which is not checked at all at the moment).

By enabling the OTel logs and traces data streams which are using LogsDB by default, ES always executes this fallback check.

This trips the following test that assert that no features should be used by starting up with default configuration:

@martijnvg
Copy link
Member

So synthetic source usage will be checked once this PR gets merged: #113759

But I think for index template we should use: SYNTHETIC_SOURCE_FEATURE.checkWithoutTracking(...) instead of SYNTHETIC_SOURCE_FEATURE.check(...). The later should only be used when an actual index gets created.

I can work on a PR for that.

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Sep 30, 2024
The SyntheticSourceLicenseService should only track if usage is allowed and an index will actually be created.

Relates to elastic#113468
@felixbarny
Copy link
Member

@gregkalapos: there are also a bunch of tests failing in native-multi-node-tests. See https://gradle-enterprise.elastic.co/s/55fntou6anahm.

@axw has disabled the apm_data plugin in these tests in the PR that enables the apm_data plugin by default (#108860)

setting 'xpack.apm_data.enabled', 'false'

So you'll probably need to make a similar change for this, too.

martijnvg added a commit that referenced this pull request Sep 30, 2024
The SyntheticSourceLicenseService should only track if usage is allowed and an index will actually be created.

Relates to #113468
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Sep 30, 2024
The SyntheticSourceLicenseService should only track if usage is allowed and an index will actually be created.

Relates to elastic#113468
elasticsearchmachine pushed a commit that referenced this pull request Sep 30, 2024
The SyntheticSourceLicenseService should only track if usage is allowed and an index will actually be created.

Relates to #113468
@felixbarny felixbarny linked an issue Oct 1, 2024 that may be closed by this pull request
@gregkalapos gregkalapos requested a review from a team as a code owner October 1, 2024 12:20
@gregkalapos
Copy link
Contributor Author

@elasticmachine update branch

@gregkalapos gregkalapos added the :Data Management/Data streams Data streams and their lifecycles label Oct 1, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Oct 1, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Oct 1, 2024
@gregkalapos gregkalapos merged commit 644fd04 into elastic:main Oct 1, 2024
16 checks passed
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

gregkalapos added a commit to gregkalapos/elasticsearch that referenced this pull request Oct 1, 2024
* Make xpack.otel_data.registry.enabled default to true

* Update build.gradle

* Disable otel-data plugin for tests where it's not relevant

---------

Co-authored-by: Elastic Machine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Oct 1, 2024
)

* Make xpack.otel_data.registry.enabled default to true

* Update build.gradle

* Disable otel-data plugin for tests where it's not relevant

---------

Co-authored-by: Elastic Machine <[email protected]>
matthewabbott pushed a commit to matthewabbott/elasticsearch that referenced this pull request Oct 4, 2024
The SyntheticSourceLicenseService should only track if usage is allowed and an index will actually be created.

Relates to elastic#113468
matthewabbott pushed a commit to matthewabbott/elasticsearch that referenced this pull request Oct 4, 2024
* Make xpack.otel_data.registry.enabled default to true

* Update build.gradle

* Disable otel-data plugin for tests where it's not relevant

---------

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Data streams Data streams and their lifecycles >non-issue Team:Data Management Meta label for data/management team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable otel-data plugin in 8.16 by default
8 participants