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

Use correct types for source.port and source.ip #737

Merged
merged 4 commits into from
Feb 24, 2021
Merged

Use correct types for source.port and source.ip #737

merged 4 commits into from
Feb 24, 2021

Conversation

mtojek
Copy link
Contributor

@mtojek mtojek commented Feb 22, 2021 •
edited
Loading

This PR adjusts the Apache intergration to use correct types for source.port and source.ip.

What does this PR do?

Different field mappings caused conflicts.

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.

How to test this PR locally

Related issues

@mtojek mtojek self-assigned this Feb 22, 2021
@mtojek mtojek requested a review from ycombinator February 22, 2021 14:55
@elasticmachine
Copy link

elasticmachine commented Feb 22, 2021 •
edited
Loading

💚 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

Expand to view the summary

Build stats

  • Build Cause: Pull request #737 updated

  • Start Time: 2021-02-24T16:26:53.096+0000

  • Duration: 9 min 48 sec

  • Commit: 591fbc7

Test stats 🧪

Test Results
Failed 0
Passed 40
Skipped 0
Total 40

Trends 🧪

Image of Build Times

Image of Tests

ycombinator
ycombinator reviewed Feb 22, 2021
View reviewed changes
packages/apache/manifest.yml
@@ -1,7 +1,7 @@
format_version: 1.0.0
name: apache
title: Apache
version: 0.3.3
version: 0.3.4
Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to confirm that we want to bump up the version just for this change, causing it to get released immediately? The alternative would be to make this change but not release it right away so other changes can accumulate for 0.3.4 before it is released.

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 think this is a bug fix and it would be good to push the change sooner than later.

ycombinator
ycombinator reviewed Feb 22, 2021
View reviewed changes
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

Change LGTM. Just have a question about whether we want to release it right away or later.

ycombinator
ycombinator reviewed Feb 22, 2021
View reviewed changes
packages/apache/changelog.yml Outdated
- version: "0.3.4"
changes:
- description: Use correct types for `source.port` and `source.ip`
type: enhancement
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like this should be bugfix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, fixed!

@mtojek
Copy link
Contributor Author

mtojek commented Feb 22, 2021

Just have a question about whether we want to release it right away or later.

It would be great to make this PR happy first, let's wait for the fix for Elastic Agent.

ycombinator
ycombinator approved these changes Feb 22, 2021
View reviewed changes
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM. WFG.

@andresrc andresrc added Integration:apache Apache HTTP Server Team:Integrations Label for the Integrations team labels Feb 22, 2021
@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)

@mtojek
Copy link
Contributor Author

mtojek commented Feb 23, 2021

/test

@mtojek
Copy link
Contributor Author

mtojek commented Feb 23, 2021

jenkins run the tests please

@mtojek
Copy link
Contributor Author

mtojek commented Feb 23, 2021

Blocked by: elastic/beats#24163

@mtojek mtojek merged commit a463ac8 into elastic:master Feb 24, 2021
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
@mtojek
Use correct types for source.port and source.ip (elastic#737)
8ec8bdc 
* Use correct types for source.port and source.ip

* Add changelog entry

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

@ycombinator ycombinator ycombinator approved these changes

@andresrc andresrc andresrc approved these changes

Assignees

@mtojek mtojek

Labels
Integration:apache Apache HTTP Server Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Wrong mappings in Apache Integration "apache.error" (7.11.1)
4 participants
@mtojek @elasticmachine @ycombinator @andresrc