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

Collect cloud metadata #1815

Closed
felixbarny opened this issue Aug 25, 2020 · 4 comments
Closed

Collect cloud metadata #1815

felixbarny opened this issue Aug 25, 2020 · 4 comments
Assignees
Labels
agent-nodejs Make available for APM Agents project planning. agent-spec Related to a cross agent functionality spec. cross APM agents enhancement
Milestone

Comments

@felixbarny
Copy link
Member

felixbarny commented Aug 25, 2020

see elastic/apm#290

see also current spec doc: https://github.com/elastic/apm/blob/3f224a9cbacb196bbb196b0868d9fa90a0bbaf6c/specs/agents/metadata.md#cloud-provider-metadata

@JCMais
Copy link
Contributor

JCMais commented Aug 26, 2020

How would someone go about implementing this?

From what I can see the metadata itself will need to be added to

// metadata
agentName: 'nodejs',
agentVersion: version,
serviceName: conf.serviceName,
serviceNodeName: conf.serviceNodeName,
serviceVersion: conf.serviceVersion,
frameworkName: conf.frameworkName,
frameworkVersion: conf.frameworkVersion,
globalLabels: maybePairsToObject(conf.globalLabels),
hostname: conf.hostname,
environment: conf.environment,
(which means adding it to the apm-nodejs-http-client package)

net.createConnection() is probably the fastest way to check if the metadata server exists, if it does a request can be made to retrieve the information. This is basically what was done on the reference implementation: https://github.com/basepi/apm-agent-python/blob/f13110156e57eec99134a7f9342243416b969698/elasticapm/utils/cloud.py#L79-L115

But the biggest "issue" so far, is that the config generation is synchronous.

A possible solution to the above that does not require a big refactor would be to create the "cloud metadata fetcher" as a function that accepts a callback and then pass that to ElasticAPMHttpClient. Writing to the stream could then be deferred until the callback has been called: https://github.com/elastic/apm-nodejs-http-client/blob/1526dedc26392bd92f20b5079f848c7e77567e7c/index.js#L496-L500

Something like:

    // All requests to the APM Server must start with a metadata object
    const encodeMetadataAndWriteStream = () => {
      if (!client._encodedMetadata) {
        client._encodedMetadata = client._encode({ metadata: client._conf.metadata }, Client.encoding.METADATA)
      }

      stream.write(client._encodedMetadata)
    }

    const { metadata } = client._conf

    if (metadata._cloudFetcher && !metadata.cloud) {
      metadata._cloudFetcher((cloud, _error) => {
        // do something with error?
        metadata.cloud = cloud
        encodeMetadataAndWriteStream()
      })
    } else {
      encodeMetadataAndWriteStream()
    }

@felixbarny
Copy link
Member Author

That sounds like a good plan and is in line with what the spec says:

Any intake API requests to the APM server should be delayed until this metadata is available.

Any events that are created before the metadata is ready would be buffered in the meantime.

@JCMais JCMais mentioned this issue Aug 26, 2020
5 tasks
@JCMais
Copy link
Contributor

JCMais commented Aug 26, 2020

I've started working on this at #1816

@astorm astorm added the agent-spec Related to a cross agent functionality spec. label Oct 8, 2020
@trentm trentm modified the milestones: 7.10, 7.11 Nov 16, 2020
@AlexanderWert AlexanderWert added agent-nodejs Make available for APM Agents project planning. cross APM agents labels Nov 16, 2020
@trentm trentm mentioned this issue Jan 21, 2021
3 tasks
astorm added a commit that referenced this issue Jan 29, 2021
* feat: docs for cloud provider configuration

* Update docs/configuration.asciidoc

* Implements #1815

Co-authored-by: Brandon Morelli <[email protected]>

Co-authored-by: Brandon Morelli <[email protected]>
astorm added a commit to elastic/apm-nodejs-http-client that referenced this issue Jan 29, 2021
* Implements: elastic/apm-agent-nodejs#1815

* chore: save progress

* feat: moving encoded metadata setting to constructor

* feat: "rough draft" of a working PR

* chore: rename to updateEncodedMetadata

* fix: attach metadata to correct spot

* chore: rewrite comment

* chore: async all the things

* test: tests for new client methods

* feat: update event name to reflect intent

* chore: lint and proofread

* chore: move tests to existing suite to avoid argv process weirdness in jenkins

* chore: change from function to object

* test: change from function to object method

* chore: lint

* chore: reunite comment with its line

* chore: err not error

* chore: remove old comment

* chore: unasync a function
astorm added a commit that referenced this issue Feb 1, 2021
* Implements: #1815

* feat: cloud metadata request

* chore: move test file

* feat: test complete

* feat: working through test failures

* fix: tests

* chore: re-org

* feat: reorg metadata

* chore: undo test removal

* chore: remove console.log

* feat: request module

* chore: moving on up

* chore: instrumentation path

* chore: bringing over test server fixtures from other branch

* chore: lint cleanup post-elrequest

* fix: failure handlers for request

* chore: lint

* feat: add cli interface for quick testing on cloud servers

* chore: exit codes for cli

* feat: fetch metadata for all, corrdinate messages

* feat: error handling for non-json responses

* feat: adding agent configuration of cloud provider and tests for same

* chore: module constants for timeouts

* ci: new test server endpoint for v2 amazon

* feat: add v2 server tests and implementation

* fix: error handling

* chore: lint

* test: full testing of unexpected responses from metadata server

* chore: lint

* test: tests for coordination object

* chore: rename to CallbackCoordination

* chore: rename library file

* fix: cleanup timeout in coordination

* chore: lint

* fix: better pattern for no double callbacks on errors

* feat: logging, comment proofread

* feat: APM Server wants strings for everything, add type casting/checking

* chore: fix port

* chore: lint

* chore: changelog

* chore: adjust log level

* chore: add link to issue

* chore: nits -- variable name and error type export

* chore: lint, remove unused data

* fix: better pattern for request callback

* chore: variable rename

* chore: change from passing entire agent to just passing provider name

* chore: use logger arg instead of global agent instance

* chore: for loop style

* chore: cast with string objects instead of concats

* chore: not new String, String

* chore: lint

* chore: logger into other tests

* chore: more getLogger calls

* chore: lint

* feat: unified IMDSv1 and IMDSv2 method

* feat: unify aws APIs

* fix: avoid catching the callback's exceptions

* fix: removes instances of "callback function in a try" anti-pattern

* feat: refactor to use URLs, move constants into modules

* fix: 32 is not x32

* chore: lint

* fix: node 8 and URL

* feat: config normalization and testing

* feat: improve API of getMetadata... methods, baseApiOverride param

* fix: log a "shouldn't happen" situation

* chore: update comments

* chore: err vs. error

* chore: no-op logger

* chore: one last logger

* chore: lint

* chore: module path
@astorm
Copy link
Contributor

astorm commented Feb 1, 2021

With #1937 and elastic/apm-nodejs-http-client#129 landed this work is done, and we be available after the next http client and agent release.

@astorm astorm closed this as completed Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning. agent-spec Related to a cross agent functionality spec. cross APM agents enhancement
Projects
None yet
Development

No branches or pull requests

5 participants