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

[Heartbeat] Add service_name option for APM integration #19932

Merged
merged 6 commits into from
Jul 17, 2020
Merged

[Heartbeat] Add service_name option for APM integration #19932

merged 6 commits into from
Jul 17, 2020

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Jul 15, 2020 •
edited
Loading

What does this PR do?

Adds a new standard service_name option to the heartbeat config file. While possible with fields already, adding this as a first class option encourages use of this important field for integration.

First step toward elastic/uptime#220

This PR also refactors some internal bits where we were passing too many parameters already, and adding service_name would just be too much. We now pass a single larger struct for common monitor options which cleans up a lot of the code.

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 against ES with service_name in your config and see that service.name is populated.

@andrewvc andrewvc added enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.10.0 labels Jul 15, 2020
@andrewvc andrewvc requested a review from a team as a code owner July 15, 2020 01:47
@andrewvc andrewvc self-assigned this Jul 15, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (Team:Uptime)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jul 15, 2020
@andrewvc andrewvc requested a review from blakerouse July 15, 2020 01:49
@andrewvc andrewvc mentioned this pull request Jul 15, 2020
Closed
blakerouse
blakerouse approved these changes Jul 15, 2020
View reviewed changes
Copy link
Contributor

@blakerouse blakerouse 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, like the consolidation of the arguments into a struct.

Think some of the imports should be cleaned up, but not going to block you on that.

heartbeat/monitors/active/http/http_test.go Outdated Show resolved Hide resolved
heartbeat/monitors/active/icmp/icmp_test.go Outdated Show resolved Hide resolved
heartbeat/monitors/active/tcp/helpers_test.go Outdated Show resolved Hide resolved
heartbeat/monitors/monitor.go Show resolved Hide resolved
heartbeat/monitors/wrappers/monitors_test.go Outdated Show resolved Hide resolved
@andrewvc
Copy link
Contributor Author

@blakerouse fixed all the import ordering issues, will merge on green. Thanks for the review!

@andrewvc andrewvc merged commit 6197850 into elastic:master Jul 17, 2020
@andrewvc andrewvc deleted the service_name branch July 17, 2020 17:19
@andrewvc andrewvc mentioned this pull request Jul 17, 2020
Merged
6 tasks
andrewvc added a commit to andrewvc/beats that referenced this pull request Jul 17, 2020
@andrewvc
[Heartbeat] Add service_name option for APM integration (elastic#19932)
6d1dc8a 
Adds a new standard service_name option to the heartbeat config file. While possible with fields already, adding this as a first class option encourages use of this important field for integration.

First step toward elastic/uptime#220

This PR also refactors some internal bits where we were passing too many parameters already, and adding service_name would just be too much. We now pass a single larger struct for common monitor options which cleans up a lot of the code.

(cherry picked from commit 6197850)
v1v added a commit to v1v/beats that referenced this pull request Jul 20, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/fix-warni…
cf807c2 
…ngs-archive

* upstream/master: (274 commits)
  Test export commands in all Beats (elastic#20016)
  [Ingest Manager] Allow using drop path for agent build (elastic#20019)
  [DOCS] Remove include for deleted file in monitoring docs (elastic#20038)
  Removing xpack.monitoring.* settings (elastic#18608)
  [Heartbeat] Add service_name option for APM integration (elastic#19932)
  [Elastic Agent] Fix merging of fleet.yml. Add --staging to enroll cmd. (elastic#20026)
  Stricter stalebot configuration for PRs (elastic#20004)
  [ci] Favor direct mage invocation on CI (elastic#19960)
  Add cloudwatch input into Filebeat configure inputs documentation (elastic#19973)
  [Filebeat] remove delimiter \n from log line in s3 input (elastic#19972)
  [Metricbeat] Update MySQL dashboard (elastic#19913)
  Packetbeat process monitor: Ignore missing /proc/net/tcp6 (elastic#19945)
  [CI] fix MODULE variable cornercases (elastic#19985)
  Ignore timestamp in fortinet/clientendpoint and netscout/sightline (elastic#19998)
  add 7.9 to .backportrc.json (elastic#19952)
  Update internal links to external (elastic#19947)
  Remove Dynamic Script Compilations warning in Cisco module (elastic#19840)
  [Elastic Agent] Fix RPM and DEB packaging for Elastic Agent (elastic#19959)
  [Ingest Manager] Do not compare err with custom type (elastic#19980)
  Fix nanocore sum for non default intervals on Kubernetes Overview Dashboard (elastic#19675)
  ...
v1v added a commit to v1v/beats that referenced this pull request Jul 20, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/ci-read-o…
a6c5555 
…nly-mod

* upstream/master: (75 commits)
  Test export commands in all Beats (elastic#20016)
  [Ingest Manager] Allow using drop path for agent build (elastic#20019)
  [DOCS] Remove include for deleted file in monitoring docs (elastic#20038)
  Removing xpack.monitoring.* settings (elastic#18608)
  [Heartbeat] Add service_name option for APM integration (elastic#19932)
  [Elastic Agent] Fix merging of fleet.yml. Add --staging to enroll cmd. (elastic#20026)
  Stricter stalebot configuration for PRs (elastic#20004)
  [ci] Favor direct mage invocation on CI (elastic#19960)
  Add cloudwatch input into Filebeat configure inputs documentation (elastic#19973)
  [Filebeat] remove delimiter \n from log line in s3 input (elastic#19972)
  [Metricbeat] Update MySQL dashboard (elastic#19913)
  Packetbeat process monitor: Ignore missing /proc/net/tcp6 (elastic#19945)
  [CI] fix MODULE variable cornercases (elastic#19985)
  Ignore timestamp in fortinet/clientendpoint and netscout/sightline (elastic#19998)
  add 7.9 to .backportrc.json (elastic#19952)
  Update internal links to external (elastic#19947)
  Remove Dynamic Script Compilations warning in Cisco module (elastic#19840)
  [Elastic Agent] Fix RPM and DEB packaging for Elastic Agent (elastic#19959)
  [Ingest Manager] Do not compare err with custom type (elastic#19980)
  Fix nanocore sum for non default intervals on Kubernetes Overview Dashboard (elastic#19675)
  ...
andrewvc added a commit that referenced this pull request Jul 29, 2020
@andrewvc
[Heartbeat] Add service_name option for APM integration (#19932) (#20034
573cff9 
)

Adds a new standard service_name option to the heartbeat config file. While possible with fields already, adding this as a first class option encourages use of this important field for integration.

First step toward elastic/uptime#220

This PR also refactors some internal bits where we were passing too many parameters already, and adding service_name would just be too much. We now pass a single larger struct for common monitor options which cleans up a lot of the code.

(cherry picked from commit 6197850)
andrewvc added a commit to andrewvc/kibana that referenced this pull request Jul 29, 2020
@andrewvc
[Uptime] Use service.name to link from Uptime -> APM where available
6e6c10f 
With elastic/beats#19932 coming in 7.10 adding
the `service.name` ECS field is very easy. We should prefer this field
when cross linking to APM, hence this PR.

Resolves elastic/uptime#220
@andrewvc andrewvc mentioned this pull request Jul 29, 2020
Merged
7 tasks
andrewvc added a commit to elastic/kibana that referenced this pull request Jul 29, 2020
@andrewvc
[Uptime] Use service.name to link from Uptime -> APM where available (
1580a2e 
#73618)

With elastic/beats#19932 coming in 7.10 adding
the `service.name` ECS field is very easy. We should prefer this field
when cross linking to APM, hence this PR.

Resolves elastic/uptime#220
andrewvc added a commit to andrewvc/kibana that referenced this pull request Jul 29, 2020
@andrewvc
[Uptime] Use service.name to link from Uptime -> APM where available (
d267483 
elastic#73618)

With elastic/beats#19932 coming in 7.10 adding
the `service.name` ECS field is very easy. We should prefer this field
when cross linking to APM, hence this PR.

Resolves elastic/uptime#220
# Conflicts:
#	x-pack/plugins/uptime/public/lib/helper/observability_integration/get_apm_href.ts
@andrewvc andrewvc mentioned this pull request Jul 30, 2020
Merged
andrewvc added a commit to elastic/kibana that referenced this pull request Aug 5, 2020
@andrewvc
[Uptime] Use service.name to link from Uptime -> APM where available (
4a4b1e0 
#73618) (#73666)

With elastic/beats#19932 coming in 7.10 adding
the `service.name` ECS field is very easy. We should prefer this field
when cross linking to APM, hence this PR.

Resolves elastic/uptime#220
# Conflicts:
#	x-pack/plugins/uptime/public/lib/helper/observability_integration/get_apm_href.ts
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@andrewvc @melchiormoulin
[Heartbeat] Add service_name option for APM integration (elastic#19932)
d0326f7 
Adds a new standard service_name option to the heartbeat config file. While possible with fields already, adding this as a first class option encourages use of this important field for integration.

First step toward elastic/uptime#220

This PR also refactors some internal bits where we were passing too many parameters already, and adding service_name would just be too much. We now pass a single larger struct for common monitor options which cleans up a lot of the code.
andrewvc added a commit that referenced this pull request Nov 12, 2020
@andrewvc
[Heartbeat] Use service.name not service_name in configs (#20330)
425271b 
Using the nesting this way lets us handle future fields like
service.environment more cleanly if/when they become available.

See https://github.com/elastic/ecs/blob/master/rfcs/text/0002-rfc-environment.md
for the proposal to add service.environment

This is a follow-up to #19932 which has not yet been released, so this is not a breaking change.
andrewvc added a commit to andrewvc/beats that referenced this pull request Nov 12, 2020
@andrewvc
[Heartbeat] Use service.name not service_name in configs (elastic#20330)
25c564a 
Using the nesting this way lets us handle future fields like
service.environment more cleanly if/when they become available.

See https://github.com/elastic/ecs/blob/master/rfcs/text/0002-rfc-environment.md
for the proposal to add service.environment

This is a follow-up to elastic#19932 which has not yet been released, so this is not a breaking change.

(cherry picked from commit 425271b)
@andrewvc andrewvc mentioned this pull request Nov 12, 2020
Merged
andrewvc added a commit that referenced this pull request Nov 12, 2020
@andrewvc
[Heartbeat] Use service.name not service_name in configs (#20330) (#2…
166761e 
…2574)

Using the nesting this way lets us handle future fields like
service.environment more cleanly if/when they become available.

See https://github.com/elastic/ecs/blob/master/rfcs/text/0002-rfc-environment.md
for the proposal to add service.environment

This is a follow-up to #19932 which has not yet been released, so this is not a breaking change.

(cherry picked from commit 425271b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blakerouse blakerouse blakerouse approved these changes

Assignees

@andrewvc andrewvc

Labels
enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrewvc @elasticmachine @blakerouse