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

[DevOps] Elastic APM #580

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

[DevOps] Elastic APM #580

wants to merge 4 commits into from

Conversation

turnUpTheChill
Copy link

@turnUpTheChill turnUpTheChill commented Apr 29, 2020 •
edited by MooseQuest
Loading

#566

Changelog entry

Added annotations and alterations to methods to allow them to be instrumented with Elastic APM.
There is additional functionality like error message logging, which can be added but this gets the base line instrumentation and metrics

Additional info

  • Anything else reviewers and future readers should know
    This is noted in the APM.md, but in order for this to work there must be an active APM server available and configured with environment variables

Pull Request Checklist

For Reviewer

  • Submitted under the correct tag
  • Runs without error or warning
  • Documentation revised / included
  • Unit tests included
  • Issues referenced
  • Does not create new bugs / issues

@turnUpTheChill turnUpTheChill changed the title Ahil elastic [DevOps] Elastic APM Apr 29, 2020
@MooseQuest MooseQuest requested a review from jlubken April 29, 2020 19:27
jlubken
jlubken requested changes Apr 29, 2020
View reviewed changes
Copy link
Collaborator

@jlubken jlubken 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 looks good.

When I run this locally I get a stream of errors where the app is attempting to connect to the metrics server.

Is there a way to prevent this (maybe with an env variable or standard convention) if the metrics server is not expected to be running?

@turnUpTheChill
Copy link
Author

@jlubken That should be possible, let me see how I can accomplish that.

We also have adocker-compose.yml file that can allow for a local monitoring stack to be deployed a long side the local app. I can provide instructions for that if it sounds like something that would be useful

@turnUpTheChill
Copy link
Author

@jlubken It looks like the capability to do this will be available in a future release with an environment variable. If you want it sooner than that, I could try and do something with the logging object
elastic/apm-agent-python#790

@turnUpTheChill
Copy link
Author

@jlubken I have added the configuration to disable APM locally. Should resolve that issue.

I can add some instructions and methods in case Developers do want to run APM locally so they can test out different instrumentation options. Let me know what sounds best to you.

cc: @MooseQuest

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

@jlubken jlubken jlubken requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

["Observability"] Implement APM, Template Example and Create Enhancement for other to contribute.
2 participants
@turnUpTheChill @jlubken