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

Added getting started guide for Python #321

Merged
merged 19 commits into from
Jun 16, 2023
Merged

Added getting started guide for Python #321

merged 19 commits into from
Jun 16, 2023

Conversation

pnvnd
Copy link
Collaborator

@pnvnd pnvnd commented Mar 22, 2023

No description provided.

@reese-lee
Copy link
Contributor

This PR is to add a Get Started with OTel Python tutorial to our examples repo, similar to what we have for Java.

The directories have already been broken out into Instrumented and Uninstrumented, and the draft for our docs site is being reviewed currently.

@pnvnd Can you add the load generator files to each directory as well, which will be more convenient for users depending on which tutorial they want to try out in the guide?

@CLAassistant
Copy link

CLAassistant commented May 9, 2023

CLA assistant check
All committers have signed the CLA.

@reese-lee
Copy link
Contributor

Thank you for making that change so promptly @pnvnd! A couple more things:

  1. Can you change the port to 8080 to align with the other examples?
  2. Can you confirm that when a user runs this tutorial, what they see in their NR1 will match what is shown in the following screen shots for the following signals (we are looking for the same span names, span attribute names, custom metric names, etc.):

pnvnd and others added 4 commits May 10, 2023 15:42
Standardizes the README to match the Java README
  * remove the service name from being hardcoded (I added an instruction to set service name as an env var in the README)
  * update the meter description
  * update the returned json response for a valid input
  * update the ValueError message 
  * remove the team tag `newrelic` 
  * remove the "Everything else" section, I moved the two items to the `Traces` and `Metrics` sections respectively
@reese-lee
Copy link
Contributor

Hello @pnvnd, thank you for your patience! I made a few changes to the following based on our recently written spec for the getting started demo apps:

  • README to clarify the steps and to standardize the format
  • modified app.py to:
    • remove the service name from being hardcoded (I added an instruction to set service name as an env var in the README)
    • update the meter description
    • update the returned json response for a valid input
    • update the ValueError message
    • remove the team tag newrelic
    • remove the "Everything else" section, I moved the two items to the Traces and Metrics sections respectively

There's just a few more things I could use your help with:

  1. We want the user to be able to access the endpoint "http://localhost:8080/fibonacci?n=[input]" (to follow the standard URL format for queries); currently, the endpoint is accessed as "http://localhost:8080/fibonacci/[input]".
  2. The parent span should show up as fibonacci in the traces page in NR as its own trace group (see here for example).
  3. Why are you setting the span kind? We want to avoid setting span kind manually.
  4. When an exception is raised due to an invalid input, we also want to return a JSON response ("n must be 1 <= n <= 90."); currently, it raises a 500 and "Internal Server Error" is displayed in the browser.

@reese-lee
Copy link
Contributor

Hey @pnvnd It is looking really good! I ran the instrumented version, and the only thing I really noticed about the data in NR is that the error is not captured as a span event on the span on which it occurs.

@reese-lee reese-lee merged commit 7c4e4b5 into newrelic:main Jun 16, 2023
2 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.

3 participants