-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Adjust SyntheticSourceLicenseService #116647
Adjust SyntheticSourceLicenseService #116647
Conversation
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license if before a cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used.
...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java
Show resolved
Hide resolved
Do we also need to test for a customer switching license? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a first pass and this is looking great. I left a few suggestions -- as discussed on Slack, it'd be great to convert the new IT case to use more realistic licensing primitives by extending AbstractLicensesIntegrationTestCase
I'll make another pass over the PR once that's ready.
...lugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java
Outdated
Show resolved
Hide resolved
private static final String CUTOFF_DATE_SYS_PROP_NAME = "es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date"; | ||
private static final Logger LOGGER = LogManager.getLogger(SyntheticSourceLicenseService.class); | ||
static final long DEFAULT_CUTOFF_DATE = LocalDateTime.of(2024, 12, 12, 0, 0).toInstant(ZoneOffset.UTC).toEpochMilli(); | ||
private static final long MAX_CUTOFF_DATE = LocalDateTime.of(2026, 12, 12, 0, 0).toInstant(ZoneOffset.UTC).toEpochMilli(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless this is already confirmed, lets validate with product that this is the max cut off we want -- I don't have a very good sense for that this value should be, personally.
...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java
Outdated
Show resolved
Hide resolved
...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java
Show resolved
Hide resolved
...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java
Outdated
Show resolved
Hide resolved
...lugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java
Outdated
Show resolved
Hide resolved
...lugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java
Outdated
Show resolved
Hide resolved
...lugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java
Outdated
Show resolved
Hide resolved
.../logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseServiceTests.java
Outdated
Show resolved
Hide resolved
.../logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseServiceTests.java
Show resolved
Hide resolved
Co-authored-by: Nikolaj Volgushev <[email protected]>
@salvatore-campagna No, I don't think we need to test switching to gold/platinum license. We already have this test coverage by testing going from basic to enterprise and the other way around. |
} | ||
// To allow the following patterns: metrics-apm.transaction.*, metrics-apm.service_transaction.*, metrics-apm.service_summary.*, | ||
// metrics-apm.service_destination.*, "metrics-apm.internal-* and metrics-apm.app.* | ||
if (dataStreamName != null && dataStreamName.startsWith("metrics-apm.")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felixbarny Double checking, is the pattern sufficient in selecting the apm data streams that currently use synthetic source? Synthetic source will require an enterprise license from 8.17.0, but for apm's synthetic source usage we will allow gold/platinum license for a little longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the pattern looks good. For reference, all the APM templates are in Elasticsearch: https://github.com/elastic/elasticsearch/tree/main/x-pack/plugin/apm-data/src/main/resources/index-templates
putLicense(createGoldOrPlatinumLicense(startPastCutoff)); | ||
ensureGreen(); | ||
|
||
createIndexWithSyntheticSourceAndAssertExpectedType(".profiling-stacktraces", "STORED"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should test with synthetic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this method creates an index with source mode set to synthetic. The last argument here is what the expected source mode should be.
String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); | ||
var result = provider.getAdditionalIndexSettings(indexName, dataStreamName, IndexMode.TIME_SERIES, null, null, settings, List.of()); | ||
assertThat(result.size(), equalTo(0)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests with license at a later date?
return createGoldOrPlatinumLicense(start); | ||
} | ||
|
||
static License createGoldOrPlatinumLicense(long start) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove this, pass the license string to the function above to deduplicate. Consider adding a boolean "isBeforeCutoff" to avoid passing dates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments, ptal.
💔 Backport failed
You can use sqren/backport to manually backport by running |
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used. Co-authored-by: Nikolaj Volgushev <[email protected]>
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used. Co-authored-by: Nikolaj Volgushev <[email protected]>
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used. Co-authored-by: Nikolaj Volgushev <[email protected]>
* Adjust SyntheticSourceLicenseService (#116647) Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used. Co-authored-by: Nikolaj Volgushev <[email protected]> * spotless --------- Co-authored-by: Nikolaj Volgushev <[email protected]>
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from #116647
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from elastic#116647
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used. Co-authored-by: Nikolaj Volgushev <[email protected]>
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from elastic#116647
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from elastic#116647
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from elastic#116647
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from elastic#116647
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used. Co-authored-by: Nikolaj Volgushev <[email protected]>
Indicates whether es.mapping.synthetic_source_fallback_to_stored_source.cutoff_date_restricted_override system property has been configured. A follow up from elastic#116647
Allow gold and platinum license to use synthetic source for a limited time. If the start time of a license is before the cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used.