elastic/integrationsPublic

NotificationsYou must be signed in to change notification settings
Fork545
Star318

Comments

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

Merged

harnish-crest-data merged 6 commits intoelastic:mainfrom

chavdaharnish:apache_spark-host-ip-conflicts

Sep 4, 2023

Merged

[O11y][Apache Spark] Resolve the conflicts in host.ip field#7468
harnish-crest-data merged 6 commits intoelastic:mainfrom
chavdaharnish:apache_spark-host-ip-conflicts

Conversation

Copy link

Contributor

harnish-crest-data commentedAug 21, 2023•
edited
Loading

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 reviewedtips 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'schangelog.yml file.

Related issues

Closes[O11y][Apache Spark] Conflicts in host.ip field #7407
RelatesMake Apache Spark GA #7406

resolve the conflicts in host.ip field

8fa043f

harnish-crest-data added Integration:apache_spark

Apache Spark

bugfixPull request that fixes a bug issue labels

Aug 21, 2023

harnish-crest-data self-assigned this

Aug 21, 2023

harnish-crest-data requested a review froma team as acode owner

August 21, 2023 07:06

update pr link in changelog.yml

d3f0901

Copy link

elasticmachine commentedAug 21, 2023•
edited
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

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.

Copy link

elasticmachine commentedAug 21, 2023•
edited
Loading

🌐 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`)	💚

harnish-crest-data requested a review frommilan-elastic

August 22, 2023 11:18

milan-elastic approved these changes

Aug 22, 2023

View reviewed changes

Copy link

Contributor

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

address review comments

31770c3

Copy link

Contributor

muthu-mps commentedAug 29, 2023

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

Copy link

ContributorAuthor

harnish-crest-data commentedAug 29, 2023

ECS fields

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

harnish-crest-data requested a review frommuthu-mps

August 29, 2023 05:21

ishleenk17 reviewed

Aug 30, 2023

View reviewed changes

packages/apache_spark/_dev/build/docs/README.md Outdated


		### 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

Member

ishleenk17Aug 30, 2023

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

Contributor

lalit-satapathyAug 30, 2023

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?

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

0fd9da0

ishleenk17 reviewed

Sep 4, 2023

View reviewed changes

packages/apache_spark/data_stream/application/fields/ecs.yml

		name:ecs.version
		name:event.type
		- external: ecs
		name:tags

Copy link

Member

ishleenk17Sep 4, 2023

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

ContributorAuthor

harnish-crest-dataSep 4, 2023

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-crest-data added2 commits

September 4, 2023 16:08

address review comments

a518165

Merge branch 'main' ofhttps://github.com/harnish-elastic/integrations…

2d8af94

…into apache_spark-host-ip-conflictsConflicts:packages/apache_spark/changelog.yml

harnish-crest-data requested a review fromishleenk17

September 4, 2023 10:47

ishleenk17 approved these changes

Sep 4, 2023

View reviewed changes

Copy link

Member

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 approved these changes

Sep 4, 2023

View reviewed changes

Copy link

Contributor

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-crest-data merged commit6569314 intoelastic:main

Sep 4, 2023

Copy link

elasticmachine commentedSep 4, 2023

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

Labels

bugfix

Pull request that fixes a bug issue

Integration:apache_spark

Apache Spark

6 participants

Movatterモバイル変換

Comments

Conversation

harnish-crest-data commentedAug 21, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

What does this PR do?

Checklist

Related issues

Uh oh!

elasticmachine commentedAug 21, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

Uh oh!

elasticmachine commentedAug 21, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

🌐 Coverage report

Uh oh!

milan-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

muthu-mps commentedAug 29, 2023

Uh oh!

harnish-crest-data commentedAug 29, 2023

Uh oh!

ishleenk17Aug 30, 2023

Choose a reason for hiding this comment

Uh oh!

lalit-satapathyAug 30, 2023

Choose a reason for hiding this comment

Uh oh!

ishleenk17Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

harnish-crest-dataSep 4, 2023

Choose a reason for hiding this comment

Uh oh!

ishleenk17 left a comment

Choose a reason for hiding this comment

Uh oh!

muthu-mps left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commentedSep 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

harnish-crest-data commentedAug 21, 2023•
edited
Loading

elasticmachine commentedAug 21, 2023•
edited
Loading

elasticmachine commentedAug 21, 2023•
edited
Loading