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

[extension/solarwindsapmsettingsextension] Overall structure of the new component #30153

Merged

Conversation

jerrytfleung
Copy link
Contributor

Description:

  • Adding the overall structure of the new component extension/solarwindsapmsettingsextension for the first PR

Link to tracking Issue:
27668

Testing:

  • Added config test
  • Added basic factory test and extension test

Documentation:

  • Updated the README.md

Copy link

linux-foundation-easycla bot commented Dec 21, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the cmd/otelcontribcol otelcontribcol command label Dec 21, 2023
@jerrytfleung jerrytfleung force-pushed the feature/solarwindsapmsettingsextension branch from da1c31a to 3d1596e Compare December 21, 2023 03:54
@jerrytfleung jerrytfleung marked this pull request as ready for review December 21, 2023 03:55
@jerrytfleung jerrytfleung requested review from a team and jpkrohling December 21, 2023 03:55
@atoulme atoulme assigned atoulme and unassigned andrzej-stencel Dec 21, 2023
@atoulme atoulme added the Accepted Component New component has been sponsored label Dec 21, 2023
@jerrytfleung
Copy link
Contributor Author

Run the linting expectations and it is looking good. Can project maintainer help to approve to rerun workflows?

linting_expectation.log

@jerrytfleung
Copy link
Contributor Author

@atoulme, thanks for approving the PR! I noticed there is a failing checks build-and-test / checks (pull_request). I will regenerate the contrib.yaml file using make gendistributions and update the PR.

@jerrytfleung
Copy link
Contributor Author

@atoulme, thanks for approving the PR! I noticed there is a failing checks build-and-test / checks (pull_request). I will regenerate the contrib.yaml file using make gendistributions and update the PR.

DONE

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label Jan 3, 2024
@atoulme atoulme removed the ready to merge Code review completed; ready to merge by maintainers label Jan 10, 2024
@atoulme
Copy link
Contributor

atoulme commented Jan 10, 2024

This PR does the job of adding the structure of the component, it passes all checks such as adding it to the versions, github codeowners, factory and so on. The issue has a discussion ongoing on whether this component is applicable to the collector: #27668 (comment)

I am going to remove the "ready to merge" label and add "on-hold" until the discussion is resolved.

@atoulme atoulme added the on hold This is blocked by another PR/issue label Jan 10, 2024
@andrzej-stencel
Copy link
Member

The issue has a discussion ongoing on whether this component is applicable to the collector: #27668 (comment)

I believe the discussion is resolved now (at least from my point of view) in favor of the extension being included in the collector. I suppose we can move forward with this PR then! 🎉

@atoulme atoulme removed the on hold This is blocked by another PR/issue label Jan 12, 2024
@atoulme
Copy link
Contributor

atoulme commented Jan 12, 2024

Please rebase so we can make sure you're good to merge.

@jerrytfleung jerrytfleung force-pushed the feature/solarwindsapmsettingsextension branch from 0c18059 to 6165d23 Compare January 13, 2024 02:07
@jerrytfleung
Copy link
Contributor Author

Please rebase so we can make sure you're good to merge.

Done. Thank you!

@jerrytfleung jerrytfleung force-pushed the feature/solarwindsapmsettingsextension branch 3 times, most recently from fda10ee to a14216a Compare January 16, 2024 18:32
@jerrytfleung jerrytfleung force-pushed the feature/solarwindsapmsettingsextension branch from a14216a to a676bcb Compare January 17, 2024 18:05
@jerrytfleung
Copy link
Contributor Author

@atoulme Rebased again. Please help to take a look. Thanks!

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

Looks good to me

@jerrytfleung
Copy link
Contributor Author

Looks good to me

Thanks! Can we merge this PR?

@dmitryax dmitryax merged commit c89b81a into open-telemetry:main Jan 23, 2024
85 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 23, 2024
@jerrytfleung jerrytfleung deleted the feature/solarwindsapmsettingsextension branch January 25, 2024 00:46
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Feb 1, 2024
…ew component (open-telemetry#30153)

**Description:** 
- Adding the overall structure of the new component
`extension/solarwindsapmsettingsextension` for the first PR

**Link to tracking Issue:** [27668](open-telemetry#27668)

**Testing:** 
- Added config test
- Added basic factory test and extension test

**Documentation:**
- Updated the README.md

---------

Co-authored-by: Antoine Toulme <antoine@toulme.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Component New component has been sponsored cmd/otelcontribcol otelcontribcol command reports/distributions/contrib.yaml
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants