-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Migrating azure-monitor-opentelemetry to azure-sdk #30089
Conversation
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @jeremydvoss.
Noted some items needed for getting this ready for making this repository its home 😃 . Most of it just adding missing files (see the template project for the general idea):
-
Add a
dev_requirements.txt
file which is used for ci/testing. I think the following contents are probably fine for now.-e ../../../tools/azure-sdk-tools -e ../../../tools/azure-devtools
along with the rest of your test requirements.
-
Add a
CHANGELOG.md
file. Formatting guidance can be found here, but pretty much formatted like:# Release History ## 1.0.0b12 (Unreleased) ### Features Added ### Breaking Changes ### Bugs Fixed ### Other Changes
-
Add an
sdk_packaging.toml
file. Can just contain:[packaging] auto_update = false
-
Add a
pyproject.toml
file with a section like:[tool.azure-sdk-build] type_check_samples = false verifytypes = false pyright = false
Can also do
pylint = false
if you want to disable that too, but better to see if you can get it to pass first. Similarly withmypy = false
. -
Add an empty
py.typed
file to the project (i.e.,./azure/monitor/opentelemetry/py.typed
) -
Add a
MANIFEST.in
file. I think contents would be:recursive-include tests *.py recursive-include samples *.py include *.md include LICENSE include azure/__init__.py include azure/monitor/__init__.py include azure/monitor/opentelemetry/py.typed
-
Can remove
setup.cfg
as it's not needed. -
Add the project to the Artifacts list in ci.yml
-
Update
https://github.com/Azure/azure-sdk-for-python/blob/main/shared_requirements.txt
with package names that are included in your setup.py dependencies but not listed there.
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting this started @jeremydvoss!
Could you please update ci.yml in the monitor root directoy to include your package artifacts?
https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/monitor/ci.yml#L32-L40
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/util/__init__.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/__init__.py
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/samples/logging/correlated_logs.py
Outdated
Show resolved
Hide resolved
Done. |
PR for vendoring: microsoft/ApplicationInsights-Python#280 |
/azp run python - monitor - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
API change check APIView has identified API level changes in this PR and created following API reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments inline that should hopefully help with the pipeline failures. Not sure why it's pulling the exporter package locally though. Will need to spend more time investigating that.
To clean up the mypy errors from the _vendor files, you can add a mypy.ini in the same level as setup.py with contents like the following:
[mypy]
warn_return_any = True
ignore_missing_imports = True
[mypy-azure.monitor.opentelemetry._vendor.*]
ignore_errors = True
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
...r/azure-monitor-opentelemetry/azure/monitor/opentelemetry/diagnostics/_diagnostic_logging.py
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this needs a rebase/merge with main with the conflict resolved so that the checks can run. Left a couple comments inline.
8d724d2
to
8a9bea1
Compare
sdk/monitor/azure-monitor-opentelemetry/tests/diagnostics/test_diagnostic_logging.py
Show resolved
Hide resolved
Ubuntu tests failing seems to stem from the following error:
|
GrahamDumpleton/wrapt#196 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the Python 3.11 compatibility for wrapt was added in 1.14.0. Can try using that as a min.
https://wrapt.readthedocs.io/en/latest/changes.html#version-1-14-0
...y/azure/monitor/opentelemetry/_vendor/v0_39b0/opentelemetry/instrumentation/asgi/__init__.py
Outdated
Show resolved
Hide resolved
941ef2a
to
78173c4
Compare
/azp run python - monitor - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
* Add azure-monitor-opentelemetry to Azure SDK * unpin api/sdk. Pull from source repo branch * Added importlib specification * Added extra quote chars * setup typo * importlib-metadata * Fix diagnostic logs tests for concurrency tests * fix status logger tests for concurrency --------- Co-authored-by: Anna Tisch <antisch@microsoft.com>
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines