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: updated Docker and CI for Datadog integration #1887

Merged
merged 42 commits into from
Jul 25, 2022

Conversation

thanhdatle
Copy link
Contributor

Problem

Datadog integration

Solution

Updated

Copy link
Member

@yong-jie yong-jie left a comment

Choose a reason for hiding this comment

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

left some initial comments. Some additional documentation in the PR detailing your changes would be appreciated if possible!

src/server/index.ts Outdated Show resolved Hide resolved
<path d="M168.84 62.08C170.127 62.08 171.17 61.0368 171.17 59.75C171.17 58.4632 170.127 57.42 168.84 57.42C167.553 57.42 166.51 58.4632 166.51 59.75C166.51 61.0368 167.553 62.08 168.84 62.08Z" stroke="#472F40" stroke-linecap="round" stroke-linejoin="round"/>
<path d="M190.38 63.91C192.495 63.91 194.21 62.1953 194.21 60.08C194.21 57.9648 192.495 56.25 190.38 56.25C188.265 56.25 186.55 57.9648 186.55 60.08C186.55 62.1953 188.265 63.91 190.38 63.91Z" stroke="#472F40" stroke-linecap="round" stroke-linejoin="round"/>
<path d="M150.56 65.7C151.326 66.9066 152.024 68.1552 152.65 69.44C153.356 70.8854 153.967 72.3752 154.48 73.9C153.442 74.8645 152.335 75.7535 151.17 76.56C149.297 77.8552 147.285 78.9354 145.17 79.78" stroke="#402A38" stroke-linecap="round" stroke-linejoin="round"/>
<svg width="490" height="153" viewBox="0 0 490 153" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Member

Choose a reason for hiding this comment

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

was this file edit intentional? seems out of scope of this datadog PR

README.md Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
@thanhdatle thanhdatle merged commit 3a3f86a into develop Jul 25, 2022
@thanhdatle thanhdatle deleted the feature/datadog-integration branch July 25, 2022 07:22
env:
REPO: ${{env.ECR_URL}}/${{env.ECR_REPO}}
TAG: ${{ needs.build-edu.outputs.tag }}
DD_API_KEY: ${{ secrets.DD_API_KEY }}
DD_ENV: ${{ env.NODE_ENV }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2022-07-25 at 12 04 16

Belated review but there's a small error here, this resolves to null as there's no NODE_ENV variable defined in this VM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be SERVERLESS_STAGE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will fix in another MR 🌮

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

Successfully merging this pull request may close these issues.

5 participants