Skip to content

Conversation

@1pkg
Copy link
Contributor

@1pkg 1pkg commented Oct 2, 2024

Motivation/summary

This PR addresses the issue #14230 and enables self instrumentation in APM Server.

Checklist

For functional changes, consider:

  • Is it observable through the addition of either logging or metrics?
  • Is its use being published in telemetry to enable product improvement?
  • Have system tests been added to avoid regression?

How to test these changes

Test that self-instrumentation is enabled by default in the Cloud.
Additionally verify that instrumentation config outlined in #13514 (comment) are handled correctly when orchestrated by the standalone agent.

Related issues

#14230

@1pkg 1pkg requested a review from a team as a code owner October 2, 2024 00:52
@mergify
Copy link
Contributor

mergify bot commented Oct 2, 2024

This pull request does not have a backport label. Could you fix it @1pkg? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-7.17 is the label to automatically backport to the 7.17 branch.
  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • backport-8.x is the label to automatically backport to the 8.x branch.

@mergify
Copy link
Contributor

mergify bot commented Oct 2, 2024

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Oct 2, 2024
@1pkg 1pkg force-pushed the enable-self-instrumentation-tracing branch from 0c20c25 to b5fc9c5 Compare October 2, 2024 00:53
@1pkg 1pkg self-assigned this Oct 2, 2024
@1pkg 1pkg changed the title Enable tracing self instrumentation in APM Server Monitoring: enable tracing self instrumentation in APM Server Oct 3, 2024
@1pkg 1pkg changed the title Monitoring: enable tracing self instrumentation in APM Server monitoring: enable tracing self instrumentation in APM Server Oct 3, 2024
Copy link
Contributor

@marclop marclop left a comment

Choose a reason for hiding this comment

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

I think this change needs a changelog entry

@1pkg 1pkg force-pushed the enable-self-instrumentation-tracing branch from cc8e20d to 473977a Compare October 4, 2024 01:16
@1pkg
Copy link
Contributor Author

1pkg commented Oct 5, 2024

While testing the changes manually, I discovered a bug in EA tracing sampling changes where sampling_rate is formatted as a binary (hex) with exponent instead of decimal literal without exponent, this breaks subsequent call in apm-agent-go when tracer is initialized with a custom sampling rate.

image

@1pkg
Copy link
Contributor Author

1pkg commented Oct 5, 2024

Raised an issue in the agent repo elastic/elastic-agent#5711

@1pkg 1pkg requested review from axw and marclop October 12, 2024 00:07
axw
axw previously approved these changes Oct 13, 2024
Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Thanks @1pkg!

@1pkg
Copy link
Contributor Author

1pkg commented Oct 14, 2024

Can't be merged yet due to the Cloud configuration missing sensible default for sampling_rate.

@1pkg 1pkg added the backport-8.16 Automated backport with mergify label Oct 18, 2024
@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2024

This pull request is now in conflicts. Could you fix it @1pkg? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b enable-self-instrumentation-tracing upstream/enable-self-instrumentation-tracing
git merge upstream/main
git push upstream enable-self-instrumentation-tracing

@1pkg 1pkg removed the request for review from marclop October 22, 2024 17:35
@1pkg 1pkg force-pushed the enable-self-instrumentation-tracing branch from ae082fb to e9040da Compare October 22, 2024 17:40
@1pkg 1pkg force-pushed the enable-self-instrumentation-tracing branch from e9040da to b4ba6d1 Compare October 22, 2024 19:49
@1pkg 1pkg merged commit 56228ce into main Oct 22, 2024
13 checks passed
@1pkg 1pkg deleted the enable-self-instrumentation-tracing branch October 22, 2024 20:24
mergify bot pushed a commit that referenced this pull request Oct 22, 2024
mergify bot pushed a commit that referenced this pull request Oct 22, 2024
mergify bot added a commit that referenced this pull request Oct 22, 2024
#14424)

(cherry picked from commit 56228ce)

Co-authored-by: Kostiantyn Masliuk <1pkg@protonmail.com>
mergify bot added a commit that referenced this pull request Oct 22, 2024
#14425)

(cherry picked from commit 56228ce)

Co-authored-by: Kostiantyn Masliuk <1pkg@protonmail.com>
@1pkg 1pkg mentioned this pull request Oct 22, 2024
11 tasks
@lahsivjar
Copy link
Contributor

lahsivjar commented Oct 29, 2024

Testing on ESS

Tested successfully with 8.16 BC2, can visualize traces (screenshots attached) Screenshot 2024-10-29 at 10 27 37 PM Screenshot 2024-10-29 at 10 36 18 PM

Testing on Standalone

Tested successfully with commit SHA ae423f0

Ran the elastic-agent locally with the following docker-compose and elastic-agent.yml:

version: '3.9'
services:
  elastic-agent:
    image: elastic-agent-systemtest:8.16.0-0e756d08-SNAPSHOT
    networks:
      - network1
    ports:
      - 8220:8220
      - 8200:8200
    user: root
    environment:
      FLEET_SERVER_ENABLE: 'true'
      FLEET_SERVER_ELASTICSEARCH_HOST: http://elasticsearch:9200
      FLEET_SERVER_SERVICE_TOKEN: AAEAAWVsYXN0aWMvZmxlZXQtc2VydmVyL3Rva2VuLTE3MzAyNDE5NTk0OTA6bFo5TnNWMlFST3VPQmp1Q1MxVC1kQQ
      FLEET_SERVER_POLICY_ID: fleet-server-policy
      FLEET_SERVER_PORT: 8220
networks:
  network1:
    name: apm-server_default
    external: true
fleet:
  enabled: true

agent.monitoring:
  # enabled turns on monitoring of running processes
  enabled: true
  traces: true
  apm:
    environment: "816bctesting"
    global_labels:
      foo: localbctesting
    hosts:
      - https://88e3f5<REDACTED>.elastic-cloud.com:443 # this is a personal ESS cluster where I am sending monitoring data
    secret_token: <REDACTED>
    sampling_rate: 1

outputs:
  default:
    type: elasticsearch
    hosts:
      - 'http://elasticsearch:9200'
    username: admin
    password: changeme

Observed traces on 88e3f5 (configured monitoring cluster):

Screenshot 2024-10-29 at 11 16 57 PM Screenshot 2024-10-29 at 11 17 33 PM

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

Labels

backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants