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.

Sign up for GitHub

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

Jump to bottom

[O11y][Apache Spark] Resolve the conflicts in host.ip field #7468

Merged
merged 6 commits into from Sep 4, 2023
Merged

[O11y][Apache Spark] Resolve the conflicts in host.ip field #7468

merged 6 commits into from Sep 4, 2023

Conversation

harnish-elastic
Copy link
Contributor

@harnish-elastic harnish-elastic commented Aug 21, 2023 •
edited

  • Bug

What does this PR do?

  • Added host.ip field in ecs.yml and added re-indexing steps to resolve conflicts for existing users.
  • ECS fields are being used in the pipeline of the particular data streams, but in the ecs.yml file the used fields are not mentioned. Hence need to update the ecs.yml file with additional fields and sort the ecs.yml in alphabetical order.

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.

Related issues

@harnish-elastic harnish-elastic self-assigned this Aug 21, 2023
@harnish-elastic harnish-elastic requested a review from a team as a code owner August 21, 2023 07:06
@elasticmachine
Copy link

elasticmachine commented Aug 21, 2023 •
edited

💚 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-09-04T10:47:06.920+0000

  • Duration: 19 min 16 sec

Test stats 🧪

Test Results
Failed 0
Passed 15
Skipped 0
Total 15

🤖 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 Aug 21, 2023 •
edited

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (0/0) 💚
Files 100.0% (0/0) 💚 3.488
Classes 100.0% (0/0) 💚 3.488
Methods 75.0% (12/16) 👎 -16.96
Lines 100.0% (0/0) 💚 7.552
Conditionals 100.0% (0/0) 💚

milan-elastic
milan-elastic approved these changes Aug 22, 2023
View reviewed changes
Copy link
Contributor

@milan-elastic milan-elastic left a comment

Choose a reason for hiding this comment

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

LGTM

@muthu-mps
Copy link
Contributor

@harnish-elastic - Update the description on the reason for adding additional ECS fields and the context for renaming a few ECS fields.

@harnish-elastic
Copy link
Contributor Author

  • ECS fields

@muthu-mps Updated the description here. Also regarding renaming of ecs fields, I have just sorted the ecs.yml file in the alphabetical order and not renamed any ecs fields!

ishleenk17
ishleenk17 reviewed Aug 30, 2023
View reviewed changes
packages/apache_spark/_dev/build/docs/README.md Outdated
@@ -63,6 +63,100 @@ Restart Spark master.

Follow the same set of steps for Spark Worker, Driver and Executor.

### Troubleshooting

If host.ip is shown conflicted under ``metrics-*`` data view, then this issue can be solved by reindexing the ``Application``, ``Driver``, ``Executor`` and ``Node`` data stream's indices.
Copy link
Contributor

Choose a reason for hiding this comment

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

The reindexing step is important for this change, but I am discussing it within team if we need to give this long troubleshooting steps in the README.

cc: @lalit-satapathy

Copy link
Collaborator

Choose a reason for hiding this comment

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

The reindexing step is important for this change, but I am discussing it within team if we need to give this long troubleshooting steps in the README.

Long term, we can explore remove such repetitive sections in the package Readme and capture in a different way?

ishleenk17
ishleenk17 reviewed Sep 4, 2023
View reviewed changes
packages/apache_spark/data_stream/application/fields/ecs.yml
- external: ecs
name: tags
Copy link
Contributor

Choose a reason for hiding this comment

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

any advantage of changing the ordering of this mapping ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one of the best practices to have field names in the alphabetical order for readability.

@harnish-elastic
address review comments
a518165
@harnish-elastic
Merge branch 'main' of https://github.com/harnish-elastic/integrations …
2d8af94 
…into apache_spark-host-ip-conflicts

Conflicts:
	packages/apache_spark/changelog.yml
ishleenk17
ishleenk17 approved these changes Sep 4, 2023
View reviewed changes
Copy link
Contributor

@ishleenk17 ishleenk17 left a comment

Choose a reason for hiding this comment

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

Looks good!

muthu-mps
muthu-mps approved these changes Sep 4, 2023
View reviewed changes
Copy link
Contributor

@muthu-mps muthu-mps left a comment

Choose a reason for hiding this comment

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

Looks good!

@harnish-elastic harnish-elastic merged commit 6569314 into elastic:main Sep 4, 2023
4 checks passed
@elasticmachine
Copy link

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

gizas pushed a commit that referenced this pull request Sep 5, 2023
@harnish-elastic @gizas
[O11y][Apache Spark] Resolve the conflicts in host.ip field (#7468)
29ff405 
* resolve the conflicts in host.ip field

* update pr link in changelog.yml

* address review comments

* remove re-indexing steps and provided the re-indexing steps link

* address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lalit-satapathy lalit-satapathy lalit-satapathy left review comments

@muthu-mps muthu-mps muthu-mps approved these changes

@ishleenk17 ishleenk17 ishleenk17 approved these changes

@milan-elastic milan-elastic milan-elastic approved these changes

Assignees

@harnish-elastic harnish-elastic

Labels
bugfix integration:ApacheSpark
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[O11y][Apache Spark] Conflicts in host.ip field
6 participants
@harnish-elastic @elasticmachine @muthu-mps @lalit-satapathy @ishleenk17 @milan-elastic