Skip to content
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

[Azure] [Container-service] Add dimension and metric_type metadata to the container_service datastream #7139

Merged

Conversation

tetianakravchenko
Copy link
Contributor

@tetianakravchenko tetianakravchenko commented Jul 25, 2023

What does this PR do?

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

…stream

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
… using wildcard

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@tetianakravchenko tetianakravchenko requested a review from a team as a code owner July 25, 2023 14:34
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@tetianakravchenko tetianakravchenko changed the title [Azure] [Container-service] [Azure] [Container-service] Add dimension and metric_type metadata to the container_service datastream Jul 25, 2023
@elasticmachine
Copy link

elasticmachine commented Jul 25, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-08-08T13:58:18.496+0000

  • Duration: 15 min 34 sec

Test stats 🧪

Test Results
Failed 0
Passed 134
Skipped 0
Total 134

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Jul 25, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (3/3) 💚
Files 100.0% (3/3) 💚
Classes 100.0% (3/3) 💚
Methods 73.333% (22/30) 👍 7.018
Lines 100.0% (21/21) 💚 4.0
Conditionals 100.0% (0/0) 💚

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@mlunadia
Copy link

@bturquet can you help us getting these reviewed? We have a long queue of Azure packages waiting on it. Merci

type: keyword
dimension: true
description: Node name
- name: status
Copy link
Contributor

Choose a reason for hiding this comment

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

Is status really required? Can a pod/container be in two different status in a given time series?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you are right, available values for this field are: True (or true) and False(or false). I will remove dimension from this

- name: condition
type: keyword
dimension: true
description: The container name
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check the description. Does condition and status field. Both has same description. Is it correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

What value goes under condition to promote it as a dimension field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

possible values of condition:
Screenshot 2023-08-02 at 14 50 38

Note: that those conditions are for the same node

@agithomas
Copy link
Contributor

Should application.id and namespace be considered as dimensions?

@tetianakravchenko
Copy link
Contributor Author

tetianakravchenko commented Aug 2, 2023

Should application.id and namespace be considered as dimensions?

@agithomas
I wasn't able to get application_id field for this datastream. It is also not available in sample_event

@zmoog could it be that this field was just copied from other datastreams, but it is not suppose to be here?

regarding the namespace field - since all documents for this datastream have the same namespace - Microsoft.ContainerService/managedClusters I haven't added it

@zmoog
Copy link
Contributor

zmoog commented Aug 2, 2023

Should application.id and namespace be considered as dimensions?

I wasn't able to get application_id field for this datastream. It is also not available in sample_event

@zmoog could it be that this field was just copied from other datastreams, but it is not suppose to be here?

This field mapping is included in all metrics data stream, including the Billing and Application Insights and Application State.

I see the Application Insights' sample_event.json file has a application_id field, it is set by the metricset, and it makes sense since it's an APM-esque service.

There's a good chance azure.application_id comes from a copy & paste. Should I create an issue to remove this mapping?

…on: true for the azure.dimwnsions.status field; fix changelog merge conflict

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@tetianakravchenko
Copy link
Contributor Author

@agithomas I've added missing dimensions, can you please have another look on this PR?

Copy link
Contributor

@agithomas agithomas left a comment

Choose a reason for hiding this comment

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

LGTM!

@tetianakravchenko tetianakravchenko merged commit 0c88f39 into elastic:main Aug 10, 2023
4 checks passed
@tetianakravchenko tetianakravchenko deleted the azure-container-service-tsdb branch August 10, 2023 13:44
@elasticmachine
Copy link

Package azure_metrics - 1.0.19 containing this change is available at https://epr.elastic.co/search?package=azure_metrics

gizas pushed a commit that referenced this pull request Sep 5, 2023
… the container_service datastream (#7139)

* Add dimension and metric_type metadata to the container_service data_stream

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* rename dimensions.* -> dimensions

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* add list of metrics to the azure.container_service object, instead of using wildcard

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* fix pr link and changelog

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* revert version

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* remove metrics.* fields; add agent.id as a dimension

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* push fields documentation changes

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

---------

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants