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

[AWS Farget] Set dimension fields #6733

Merged
merged 5 commits into from Jul 7, 2023

Conversation

constanca-m
Copy link
Contributor

@constanca-m constanca-m commented Jun 28, 2023

What does this PR do?

Set dimension fields for TSDB migration.

Details

ECS dimensions: The rationale for the defined ECS dimensions can be found here. The fields for this specific data stream would be:

  • agent.id
  • cloud.account.id
  • cloud.region

For the specific fields, we would need these three:

  • awsfargate.task_stats.cluster_name
  • awsfargate.task_stats.identifier: each task is uniquely identified by the ID within a cluster
  • Edit: container.name is added as dimension in case there is more than one container in a task.

Since ECS has a field that clusters four of them (account.id and region, cluster_name and identifier), container.labels.com_amazonaws_ecs_task-arn, we set that one as dimension instead of all four. The structure for that field is: arn:aws:ecs:<region>:<account-id>:task/<cluster-name>/<task-id>.

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.

Related issues

Relates to #6732.

Signed-off-by: constanca-m <constanca.manteigas@elastic.co>
@constanca-m constanca-m requested a review from a team as a code owner June 28, 2023 09:46
Signed-off-by: constanca-m <constanca.manteigas@elastic.co>
@elasticmachine
Copy link

elasticmachine commented Jun 28, 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-07-07T07:33:32.783+0000

  • Duration: 15 min 40 sec

Test stats 🧪

Test Results
Failed 0
Passed 3
Skipped 0
Total 3

🤖 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 Jun 28, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (0/0) 💚
Files 100.0% (0/0) 💚
Classes 100.0% (0/0) 💚
Methods 66.667% (2/3) 👎 -33.333
Lines 100.0% (0/0) 💚 29.276
Conditionals 100.0% (0/0) 💚

Signed-off-by: constanca-m <constanca.manteigas@elastic.co>
@@ -1,4 +1,9 @@
# newer versions go on top
- version: 0.2.2
changes:
- description: Set dimension fields.
Copy link
Member

Choose a reason for hiding this comment

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

This sets dimension fields, but also agent.id. Should we mention it here?

Suggested change
- description: Set dimension fields.
- description: Set `dimension`, and `agent.id` fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agent.id was already present, but this time, it is just explicitly set on the ecs.yml. I can add that. What do you think is best @dmathieu?

Copy link
Member

Choose a reason for hiding this comment

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

Mentioning that it's now explicitly set sounds good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @dmathieu , I updated the changelog to include that

Copy link
Contributor

@tetianakravchenko tetianakravchenko left a comment

Choose a reason for hiding this comment

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

@@ -33,6 +33,7 @@
name: container.id
- external: ecs
name: container.name
dimension: true
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if awsfargate.task_stats.task_name isn't a better option for this


so to rely not on the container name (in case there will be some changes in name generation), but the task name instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mention it on the description: The structure for that field is: arn:aws:ecs:<region>:<account-id>:task/<cluster-name>/<task-id>, that is why those fields are not set as dimension, because they are included in this field.

constanca-m and others added 2 commits July 7, 2023 09:32
Signed-off-by: constanca-m <constanca.manteigas@elastic.co>
@constanca-m constanca-m requested a review from dmathieu July 7, 2023 07:54
@constanca-m constanca-m merged commit 4877bec into elastic:main Jul 7, 2023
4 checks passed
@constanca-m constanca-m deleted the aws-fargate-dimensions branch July 7, 2023 08:21
@elasticmachine
Copy link

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

gizas pushed a commit that referenced this pull request Sep 5, 2023
* Set dimension fields.

Signed-off-by: constanca-m <constanca.manteigas@elastic.co>

* Update changelog.

Signed-off-by: constanca-m <constanca.manteigas@elastic.co>

* Add container.name as dimension

Signed-off-by: constanca-m <constanca.manteigas@elastic.co>

* Resolve conflicts

Signed-off-by: constanca-m <constanca.manteigas@elastic.co>

---------

Signed-off-by: constanca-m <constanca.manteigas@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

5 participants