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

Use common host parser in vsphere module #26904

Merged
merged 5 commits into from
Jul 15, 2021

Conversation

jsoriano
Copy link
Member

What does this PR do?

Use the common host parser builder to parse hosts defined in vsphere module configuration.

Why is it important?

Since #21022, sanitized URIs included in modules host data are used as service.address. vsphere did a custom parsing that didn't fill the sanitized URI and then service.address was not filled.

See #26902.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Run the vsphere module and check that documents contain a service.address field that corresponds to the sanitized (without passwords) configured host.

Related issues

Use cases

  • To be able to distinguish metrics collected from different hosts.

@jsoriano jsoriano added bug Team:Integrations Label for the Integrations team backport-v7.14.0 Automated backport with mergify backport-v7.15.0 Automated backport with mergify labels Jul 15, 2021
@jsoriano jsoriano self-assigned this Jul 15, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 15, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 15, 2021
@exekias
Copy link
Contributor

exekias commented Jul 15, 2021

ey @jsoriano I'm wondering if a more lasting solution would be to fix #21022 and fallback to msw.Host() when msw.HostData().SanitizedURI is empty. WDYT?

@exekias
Copy link
Contributor

exekias commented Jul 15, 2021

Oh I see this would be needed anyway, right? as the vsphere URL may actually need to be sanitized

@jsoriano
Copy link
Member Author

ey @jsoriano I'm wondering if a more lasting solution would be to fix #21022 and fallback to msw.Host() when msw.HostData().SanitizedURI is empty. WDYT?

I thought about that, but what value would we use? With msw.Host() or other values not explicitly sanitized we would risk to reintroduce the issues solved by #21022.

@exekias
Copy link
Contributor

exekias commented Jul 15, 2021

Yes, let's move the conversation to #26902 as it has more implications

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 15, 2021

💚 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: 2021-07-15T09:33:59.957+0000

  • Duration: 75 min 24 sec

  • Commit: 7612366

Test stats 🧪

Test Results
Failed 0
Passed 8933
Skipped 2469
Total 11402

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 8933
Skipped 2469
Total 11402

@jsoriano jsoriano merged commit 68e9909 into elastic:master Jul 15, 2021
@jsoriano jsoriano deleted the vsphere-service-address branch July 15, 2021 11:12
mergify bot pushed a commit that referenced this pull request Jul 15, 2021
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)
mergify bot pushed a commit that referenced this pull request Jul 15, 2021
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)
jsoriano added a commit that referenced this pull request Jul 15, 2021
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
jsoriano added a commit that referenced this pull request Jul 15, 2021
)

Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
mdelapenya added a commit to mdelapenya/beats that referenced this pull request Jul 19, 2021
* master: (61 commits)
  Add disk queue unit tests based on the queuetest package
  [Heartbeat] redact authorization headers from logger (elastic#26892)
  Expose custom process metrics  (elastic#26912)
  [gcp/billing] always quote table name identifier (elastic#26870)
  Add Beats central management removal to BCs (elastic#26400)
  Add custom suffix to identifiers in filestream input when needed (elastic#26669)
  Update asa-ftd-pipeline.yml (elastic#26265)
  Use common host parser in vsphere module (elastic#26904)
  [automation] Update go release version 1.16.6 (elastic#26860)
  Skip flaky test: filestream and harvester group (elastic#26728)
  [Filebeat] Remove alias fields from Suricata and Traefik module mappings (elastic#26627)
  docs: apm-server.auth (elastic#26831)
  [Automation] Update elastic stack version to 8.0.0-2f008f4a for testing (elastic#26881)
  Clarify the scope of start/end multiline example (elastic#26786)
  [Heartbeat]: update Node.js version for synthetics (elastic#26867)
  [fix][httpjson] Fix incorrect key for template data (elastic#26848)
  [httpjson] Add value_type parameter to httpjson transforms (elastic#26847)
  [Heartbeat]: capture error from journey/end events (elastic#26781)
  [Winlogbeat] Fixes for wineventlog experimental api (elastic#26826)
  Set agent.id to Fleet Agent ID for each metric/log monitoring input (elastic#26776)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.14.0 Automated backport with mergify backport-v7.15.0 Automated backport with mergify bug Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants