-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Fleet] TSDB: need metric_type mapping for dynamic templates #155004
Comments
Pinging @elastic/fleet (Team:Fleet) |
This syntax may not be valid according to the spec after elastic/package-spec#525. Alternatively, this syntax might work from the spec side:
|
@mrodm Is a package-spec change needed for this? I see you created a draft pr. |
If I understand correctly, the kibana change needed is to extend the logic for dynamic mappings here, to replace @lalit-satapathy Could you confirm if we can use these steps to verify the kibana change in this issue:
|
@juliaElastic That PR is an attempt to add more validation checks for those keys, but it should not be required for this issue. Currently, it should be possible to define the fields as detailed in #155004 (comment) |
Hey @jsoriano, I applied your solution: - name: istio.istiod.metrics.*.counter
type: double
metric_type: counter
description: >
Istiod counter metric But the field name in vertical axis disappears: The mapping for the field is with TSDB enabled is: "dynamic_templates": [
{
"istio.istiod.metrics.*.counter": {
"path_match": "istio.istiod.metrics.*.counter",
"match_mapping_type": "double",
"mapping": {
"type": "double"
}
}
},
...
],
...
"metrics": {
"properties": {
"pilot_inbound_updates": {
"properties": {
"counter": {
"type": "long"
},
... For TSDB disabled is: {
"istio.istiod.metrics.*.counter": {
"path_match": "istio.istiod.metrics.*.counter",
"mapping": {
"type": "double"
}
}
},
...
"pilot_inbound_updates": {
"properties": {
"counter": {
"type": "double"
}, Conclusion: the Also applied @juliaElastic suggestion, and it did not work. Mapping for the field stayed the same as for TSDB disabled: - name: istio.istiod.metrics.*.counter
type: object
object_type: double
object_type_mapping_type: "*"
metric_type: counter
description: >
Istiod counter metric Resulting mapping: {
"istio.istiod.metrics.*.counter": {
"path_match": "istio.istiod.metrics.*.counter",
"mapping": {
"type": "double"
}
}
},
...
"pilot_inbound_updates": {
"properties": {
"counter": {
"type": "double"
}, |
@jlind23 We have a few packages (Prometheus server, Istio, Airflow, CockroachDB, Redisenterprise) that are currently blocked for migration to TSDB due to this issue, any support you can provide is much welcomed |
@constanca-m We have to make a change in Kibana/Fleet to apply the The issue is in our current sprint and not started yet. |
@constanca-m tested suggested syntax, but it doesn't work and the visualization that use metric defined with the new syntax is broken. Should there be added a support for |
After discussing with @criamico, she agreed on taking a stab at it as soon as possible |
Currently working on this, I have a question about the change described in #155004 (comment). Should this mapping only be added if |
Yes, similar to the |
Closes #155004 ## Summary Add dynamic mapping for tsdb `time_series_metric`. The logic is replacing `metric_type` with `time_series_metric` in dynamic fields that have a wildcard in the name. <img width="2599" alt="Screenshot 2023-06-20 at 15 33 37" src="https://github.com/elastic/kibana/assets/16084106/3265ae5c-3416-4e16-a017-9c3cd3abb4ac"> ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@tetianakravchenko @lalit-satapathy The Fleet implementation for this was merged yesterday, could you do testing on your side? It is available in the latest kibana 8.9.0-SNAPSHOT. |
@juliaElastic I added this to the mapping of an object:
And used version 8.9.0 Snapshot just now. The mapping is:
So we can't see the time series metric. |
The fix for group type is merged and backported to 8.9, should be available in the next snapshot. |
@tetianakravchenko @lalit-satapathy Are all the issues resolved with |
@amolnater-qasource To test this feature:
|
Thank you for the details @juliaElastic |
@tetianakravchenko Please comment/close this issue if dynamic mapping is resolved in Prometheus. |
@juliaElastic @lalit-satapathy
but in the full mapping:
I am not sure if this issue is related to the fleet or elasticsearch itself |
@tetianakravchenko I think you're missing a dynamic_template that matches on type |
@joshdover there is no |
@tetianakravchenko are you referring to the metric in the mapping given below or some other metric?
Where this mapping comes from? |
yes, this is one of the metric, that has not expected mapping. In my comment above, you can find full mapping and notice that not all metrics unde
I believe it defined dynamically, since default configuration is |
@tetianakravchenko Could you help document the steps to reproduce the issue? I'm not seeing those |
yes, those fields are added when the data is ingested. Those metrics are scraped from the prometheus server To reproduce it, you need to install prometheus server and use it as a host for the prometheus.collector datastream configuration |
@juliaElastic I think I found the issue.
this configuration would generate
but the configuration above is not supported by the elastic-package, so we switched to this field configuration:
in the end it generate this dynamic_template:
Note the difference of So it is expected that at @lalit-satapathy I think we should switch back to the format:
|
As was discussed with @lalit-satapathy:
|
Hi Team, We have created 05 test cases for this feature under the Fleet test suite at the links:
Please let us know if we are missing any scenarios to be covered here. Thanks! |
Hi Team, We have executed 05 testcases under the Feature test run for the 8.10.0 release at the link: Status:
Build details: As the testing is completed on this feature, we are marking this as QA:Validated. Please let us know if anything else is required from our end. |
We have not seen any new issues on this either. |
We have multiple integrations which use the dynamic templates for field mappings. The Fleet generated mapping for the dynamic templates is well described in the issue here.
In TSDB to annotate, metric_type the requirement from elasticsearch is to:
This mapping is currently done via metric_type in fields.yml file.
The issue is enabling the same for dynamic templates. How would the metric_type work for the fields with dynamic templates? Based on the elastic search documentation, above I am assuming, an explicit metric_type mapping is needed for dynamic templates. Can Fleet does the mapping for the same?
For example, Fleet can respects a (metric_type: counter) as part of the dynamic templates or provide any similar alternative usage.
This requirement impacts o11y TSDB migration multiple Prometheus style packages. Short-term testing solution for testing is to add this manually in a custom template like below:
Packages affected:
@ruflin @andresrc @joshdover @kpollich
The text was updated successfully, but these errors were encountered: