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

feat: add app insights monitoring #151

Merged
merged 18 commits into from
Oct 7, 2023
Merged

feat: add app insights monitoring #151

merged 18 commits into from
Oct 7, 2023

Conversation

danstis
Copy link
Owner

@danstis danstis commented Oct 7, 2023

Checklist

  • Ensure PR title matches the conventional commits format.

CodiumAI Agent Commands

As required run the following CodiumAI Agent commands:

  • Request PR review using @CodiumAI-Agent /review (and if required, request an incremental update with @CodiumAI-Agent /review -i)
  • Request PR improvement suggestions with @CodiumAI-Agent /improve --extended
  • Request PR update using @CodiumAI-Agent /describe

…CS_EXPORTER, and OTEL_TRACES_EXPORTER environment variables to None if the APPLICATIONINSIGHTS_CONNECTION_STRING is not populated.

# Aider chat conversation:

USER: update the app class to set the following environment variables to None if the APPLICATIONINSIGHTS_CONNECTION_STRING is not populated: OTEL_LOGS_EXPORTER, OTEL_METRICS_EXPORTER, OTEL_TRACES_EXPORTER

ASSISTANT: To implement this change, we need to modify the `ado_asana_sync\sync\app.py` file. Specifically, we will update the `__init__` method of the `App` class to set the environment variables `OTEL_LOGS_EXPORTER`, `OTEL_METRICS_EXPORTER`, and `OTEL_TRACES_EXPORTER` to `None` if `APPLICATIONINSIGHTS_CONNECTION_STRING` is not populated.

Here is the *edit block* for this change:

```python
ado_asana_sync\sync\app.py
<<<<<<< HEAD
        self.applicationinsights_connection_string = (
            applicationinsights_connection_string
            or os.environ.get("APPLICATIONINSIGHTS_CONNECTION_STRING", None)
        )
        self.ado_core_client = None
        self.ado_wit_client = None
        self.ado_work_client = None
        self.asana_client = None
        self.asana_page_size = ASANA_PAGE_SIZE
        self.asana_tag_gid = None
        self.asana_tag_name = ASANA_TAG_NAME
        self.db = None
        self.matches = None
        self.sleep_time = SLEEP_TIME
=======
        self.applicationinsights_connection_string = applicationinsights_connection_string or os.environ.get("APPLICATIONINSIGHTS_CONNECTION_STRING", None)
        if not self.applicationinsights_connection_string:
            os.environ["OTEL_LOGS_EXPORTER"] = None
            os.environ["OTEL_METRICS_EXPORTER"] = None
            os.environ["OTEL_TRACES_EXPORTER"] = None
        self.ado_core_client = None
        self.ado_wit_client = None
        self.ado_work_client = None
        self.asana_client = None
        self.asana_page_size = ASANA_PAGE_SIZE
        self.asana_tag_gid = None
        self.asana_tag_name = ASANA_TAG_NAME
        self.db = None
        self.matches = None
        self.sleep_time = SLEEP_TIME
>>>>>>> updated
```

This change will ensure that the OpenTelemetry exporters are not set if the Application Insights connection string is not provided.
@danstis danstis linked an issue Oct 7, 2023 that may be closed by this pull request
@danstis danstis self-assigned this Oct 7, 2023
@danstis danstis added the enhancement New feature or request label Oct 7, 2023
@danstis
Copy link
Owner Author

danstis commented Oct 7, 2023

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link
Contributor

PR Analysis

  • 🎯 Main theme: Adding application monitoring to the app
  • 📝 PR summary: This PR introduces application monitoring by setting up environment variables for OpenTelemetry Exporters based on the presence of an Application Insights connection string. It also includes setting up logging and tracing functionality, and updates the tests accordingly.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: Yes
  • ⏱️ Estimated effort to review [1-5]: 3, because the PR involves changes in multiple files and introduces new concepts like logging and tracing which requires some domain knowledge.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR is well-structured and the changes are logically grouped. The use of OpenTelemetry for application monitoring is a good practice. However, setting environment variables within the application code is not recommended. It would be better to manage these settings externally, for example, through a configuration file or environment variables set at the system level.

  • 🤖 Code feedback:

    • relevant file: ado_asana_sync/sync/app.py
      suggestion: Instead of setting environment variables within the application code, consider managing these settings externally. This could be done through a configuration file or environment variables set at the system level. [important]
      relevant line: os.environ["OTEL_LOGS_EXPORTER"] = "None"

    • relevant file: ado_asana_sync/sync/app.py
      suggestion: It's a good practice to avoid hardcoding strings like "None". Consider defining a constant for this value. [medium]
      relevant line: os.environ["OTEL_LOGS_EXPORTER"] = "None"

    • relevant file: ado_asana_sync/sync/app.py
      suggestion: Consider using a configuration management library to handle environment variables and other configuration settings. This would make the code cleaner and easier to manage. [medium]
      relevant line: self.applicationinsights_connection_string = (

How to use

Tag me in a comment '@CodiumAI-Agent' and add one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@danstis
Copy link
Owner Author

danstis commented Oct 7, 2023

@CodiumAI-Agent /improve --extended

@danstis
Copy link
Owner Author

danstis commented Oct 7, 2023

@CodiumAI-Agent /review -i

@CodiumAI-Agent
Copy link
Contributor

Incremental PR Review

  • ⏮️ Review for commits since previous PR-Agent review: Starting from commit 248cbf7

PR Analysis

  • 🎯 Main theme: Adding application monitoring to the app
  • 📝 PR summary: This PR introduces application monitoring to the app. It sets environment variables for logs, metrics, and traces exporters to None if the Application Insights connection string is not populated. It also includes some refactoring and code improvements.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No

How to use

Tag me in a comment '@CodiumAI-Agent' and add one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@sonarcloud
Copy link

sonarcloud bot commented Oct 7, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

37.7% 37.7% Coverage
4.5% 4.5% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@danstis danstis merged commit 4d04731 into main Oct 7, 2023
5 of 6 checks passed
@danstis danstis deleted the danstis/issue126 branch October 7, 2023 10:20
@CodiumAI-Agent
Copy link
Contributor

How to use PR-Agent

Tag me in a comment '@CodiumAI-Agent' and add one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add application monitoring to the app
2 participants