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

fix(instrumentation): the build problem for watsonx auto instrumentation #885

Merged
merged 3 commits into from Apr 26, 2024

Conversation

jinsongo
Copy link
Contributor

@jinsongo jinsongo commented Apr 23, 2024

We need to use ibm-watson-machine-learning to replace ibm_watson_machine_learning in code, and the Python>=3.10 is required by ibm-watson-machine-learning version 1.0.347 and above.

  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

@CLAassistant
Copy link

CLAassistant commented Apr 23, 2024

CLA assistant check
All committers have signed the CLA.

@gyliu513
Copy link
Contributor

We need to use ibm-watson-machine-learning to replace ibm_watson_machine_learning in code, and the Python>=3.10 is required by ibm-watson-machine-learning version 1.0.347 and above.

@jinsongo can you explain more why we need to update this?

Also please sign the CLA, thanks

@gyliu513
Copy link
Contributor

@jinsongo can you also append a test case as well? Thanks

@@ -11,11 +11,12 @@ readme = "README.md"
include = "opentelemetry/instrumentation/watsonx"

[tool.poetry.dependencies]
python = ">=3.9,<4"
python = ">=3.10,<4"
Copy link
Member

Choose a reason for hiding this comment

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

We can't do this, since the SDK supports 3.9 as well and it depends on watsonx this means that the SDK won't be able to be installed on Python 3.9 (not sure why the tests didn't catch that 🤔 )

Copy link
Contributor Author

@jinsongo jinsongo Apr 24, 2024

Choose a reason for hiding this comment

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

@nirga If to set python = ">=3.9 for ibm_watson_machine_learning by force, there may be potential risks because ibm-watson-machine-learning requires Python >=3.10.

Copy link
Member

Choose a reason for hiding this comment

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

@jinsongo people won't be able to use it in Python 3.9 anyway since you can install ibm-watson-machine-learning on Python 3.9, right?

Copy link
Contributor Author

@jinsongo jinsongo Apr 24, 2024

Choose a reason for hiding this comment

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

@nirga, On Python 3.9, the highest supported version is 1.0.333
https://pypi.org/project/ibm-watson-machine-learning/1.0.335/ was yanked because of "Causes errors on py 3.9", since then Python 3.10 is required for the release 1.0.338 and above.

# python3 --version
Python 3.9.18
# pip install ibm-watson-machine-learning==1.0.347
ERROR: Could not find a version that satisfies the requirement ibm-watson-machine-learning==1.0.347
(from versions: 1.0.8, 1.0.9, 1.0.10, 1.0.14, 1.0.15, 1.0.22, 1.0.28, 1.0.29, 1.0.34, 1.0.38, 1.0.43, 1.0.44, 1.0.45, 1.0.51, 1.0.53, 1.0.57, 1.0.66, 1.0.70, 1.0.77, 1.0.79, 1.0.86, 1.0.91, 1.0.96, 1.0.99, 1.0.100, 1.0.102, 1.0.105, 1.0.115, 1.0.116, 1.0.118, 1.0.120, 1.0.122, 1.0.135, 1.0.139, 1.0.141, 1.0.142, 1.0.157, 1.0.162, 1.0.165, 1.0.166, 1.0.173, 1.0.175, 1.0.176, 1.0.178, 1.0.179, 1.0.180, 1.0.181, 1.0.189, 1.0.200, 1.0.202, 1.0.204, 1.0.206, 1.0.208, 1.0.209, 1.0.210, 1.0.214, 1.0.218, 1.0.222, 1.0.227, 1.0.229, 1.0.232, 1.0.237, 1.0.238, 1.0.244, 1.0.246, 1.0.248, 1.0.250, 1.0.253, 1.0.255, 1.0.256, 1.0.257, 1.0.260, 1.0.262, 1.0.264, 1.0.268, 1.0.272, 1.0.273, 1.0.277, 1.0.280, 1.0.283, 1.0.286, 1.0.288, 1.0.292, 1.0.293, 1.0.294, 1.0.297, 1.0.302, 1.0.307, 1.0.308, 1.0.309, 1.0.310, 1.0.311, 1.0.312, 1.0.313, 1.0.316, 1.0.319, 1.0.320, 1.0.321, 1.0.322, 1.0.324, 1.0.326, 1.0.327, 1.0.330, 1.0.333, 1.0.335)

Copy link
Member

Choose a reason for hiding this comment

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

Let's use 1.0.335 for the tests then

Copy link
Contributor

Choose a reason for hiding this comment

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

@nirga we can use https://pypi.org/project/ibm-watson-machine-learning/1.0.333/ for some test now.

Do we have any plan to upgrade the openLLMetry python version to a higher version or we want to stick to 3.9? Thanks

Copy link
Member

Choose a reason for hiding this comment

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

We want to support a range from 3.9 to 3.12. At the end of the day, this doesn't affect your instrumentation, only the tests. So I don't see any reason not to use an older version if it's just for the tests.

Copy link
Contributor Author

@jinsongo jinsongo Apr 26, 2024

Choose a reason for hiding this comment

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

@nirga Please review, the ibm-watson-machine-learning 1.0.333 is added for tests. Thanks!

@jinsongo
Copy link
Contributor Author

jinsongo commented Apr 24, 2024

  1. The right package name is ibm-watson-machine-learning, not ibm_watson_machine_learning, otherwise the build will be failed because of [ERROR: Cannot read properties of undefined (reading ‘name’)]
    To use poetry search ibm_watson_machine_learning or poetry search ibm-watson-machine-learning, the found package name is always ibm-watson-machine-learning

  2. The following error message is reported when adding ibm-watson-machine-learning dependency, the suggested solution is to use Python >=3.10,<4;

The current project's supported Python range (>=3.9,<4) is not compatible with some of the required packages Python requirement:
  - ibm-watson-machine-learning requires Python >=3.10, so it will not be satisfied for Python >=3.9,<3.10

Because opentelemetry-instrumentation-watsonx depends on ibm-watson-machine-learning (1.0.347) which requires Python >=3.10, version solving failed.

  • Check your dependencies Python requirement: The Python requirement can be specified via the `python` or `markers` properties

    For ibm-watson-machine-learning, a possible solution would be to set the `python` property to ">=3.10,<4"

@jinsongo
Copy link
Contributor Author

With the PR changes, the build can be passed for watsonx auto instrumentation:

# nx run opentelemetry-instrumentation-watsonx:build

> nx run opentelemetry-instrumentation-watsonx:build


  Building project  opentelemetry-instrumentation-watsonx ...

  Copying project files to a temporary folder
  Resolving dependencies...
Warning: poetry-plugin-export will not be installed by default in a future version of Poetry.
In order to avoid a breaking change and make your automation forward-compatible, please install poetry-plugin-export explicitly. See https://python-poetry.org/docs/plugins/#using-plugins for details on how to install a plugin.
To disable this warning run 'poetry config warnings.export false'.
    • Adding certifi==2024.2.2  dependency
    • Adding charset-normalizer==3.3.2  dependency
    • Adding deprecated==1.2.14  dependency
    • Adding ibm-cos-sdk-core==2.13.4  dependency
    • Adding ibm-cos-sdk-s3transfer==2.13.4  dependency
    • Adding ibm-cos-sdk==2.13.4  dependency
    • Adding ibm-watson-machine-learning==1.0.355  dependency
    • Adding idna==3.7  dependency
    • Adding importlib-metadata==7.0.0  dependency
    • Adding jmespath==1.0.1  dependency
    • Adding lomond==0.3.3  dependency
    • Adding numpy==1.26.4  dependency
    • Adding opentelemetry-api==1.24.0  dependency
    • Adding opentelemetry-instrumentation==0.45b0  dependency
    • Adding opentelemetry-semantic-conventions-ai==0.1.1  dependency
    • Adding opentelemetry-semantic-conventions==0.45b0  dependency
    • Adding packaging==24.0  dependency
    • Adding pandas==2.1.4  dependency
    • Adding python-dateutil==2.9.0.post0  dependency
    • Adding pytz==2024.1  dependency
    • Adding requests==2.31.0  dependency
    • Adding setuptools==69.5.1  dependency
    • Adding six==1.16.0  dependency
    • Adding tabulate==0.9.0  dependency
    • Adding tzdata==2024.1  dependency
    • Adding urllib3==1.26.18  dependency
    • Adding wrapt==1.16.0  dependency
    • Adding zipp==3.18.1  dependency
    • Resolving dependency: ibm-watson-machine-learning
    • Extra: instruments - ibm-watson-machine-learning Locked Dependencies
  Generating sdist and wheel artifacts
Running command: poetry build at /tmp/nx-python/build/7a5c8b8c-60bd-41c0-a00f-564e0b780208 folder

Creating virtualenv opentelemetry-instrumentation-watsonx in /tmp/nx-python/build/7a5c8b8c-60bd-41c0-a00f-564e0b780208/.venv
Building opentelemetry-instrumentation-watsonx (0.16.6)
  - Building sdist
  - Built opentelemetry_instrumentation_watsonx-0.16.6.tar.gz
  - Building wheel
  - Built opentelemetry_instrumentation_watsonx-0.16.6-py3-none-any.whl
  Artifacts generated at packages/opentelemetry-instrumentation-watsonx/dist folder

 ————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  NX   Successfully ran target build for project opentelemetry-instrumentation-watsonx (4s)

@nirga nirga merged commit 7878fcc into traceloop:main Apr 26, 2024
7 checks passed
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.

None yet

4 participants