Skip to content
This repository was archived by the owner on Jun 11, 2024. It is now read-only.

Conversation

@Johnius
Copy link
Collaborator

@Johnius Johnius commented Nov 22, 2019

No description provided.

@Johnius
Copy link
Collaborator Author

Johnius commented Nov 22, 2019

@30blows @snarfbeest I would appreciate it if you could take a look at this commit.

@30blows
Copy link

30blows commented Nov 26, 2019

@john - the changes look sane, though I cannot verify they work without testing. Comments:

  • A failure to find an entry in apm_id_hostname_map results in the instance ID falling back to the previous method/value; This is fine, but it would be nice to log a warning, so we have visibility into such failures.
  • Populating the apm_id_hostname_map timely for use is dependent on the order fetches are specified in the driver config. This is okay, as it may be impractical for the driver to compute an order based on inspecting the full set of queries. But it does mean that a driver config which is not properly ordered will result in (some) metrics falling back to the previous method/value. This is another reason to log a warning on missing map entry, since this may be caused by driver misconfig (user error).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants