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

Add a rich console exporter #686

Merged
merged 15 commits into from
Sep 29, 2021
Merged

Conversation

tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Sep 19, 2021

Description

This is an alternative to the Console Exporter, that uses the Rich Console library to print colorised trees from a batch span processor. Span attributes are shown as nodes in the tree and spans are children of their related parent span/trace.

This is easier to read than the console exporter and see the relationships between traces.

Useful for debugging.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Example output:

Trace 6d3953ac92706f9dcbdc7129a4c72736                                                                                              
└── [01:40:55.139608] /locations, span 503921bf52866810                                                                             
    ├── Kind : SERVER                                                                                                               
    ├── Attributes :                                                                                                                
    │   ├── http.scheme : http                                                                                                      
    │   ├── http.host : 127.0.0.1:8000                                                                                              
    │   ├── net.host.port : 8000                                                                                                    
    │   ├── http.flavor : 1.1                                                                                                       
    │   ├── http.target : /locations                                                                                                
    │   ├── http.url : http://127.0.0.1:8000/locations                                                                              
    │   ├── http.method : GET                                                                                                       
    │   ├── http.server_name : 127.0.0.1:8000                                                                                       
    │   ├── http.user_agent : Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)              
    │   │   Version/14.1.2 Safari/605.1.15                                                                                          
    │   ├── net.peer.ip : 127.0.0.1                                                                                                 
    │   ├── net.peer.port : 64673                                                                                                   
    │   ├── http.route : /locations                                                                                                 
    │   └── http.status_code : 200                                                                                                  
    ├── [01:40:55.142977] SELECT "id","name","private" FROM "locations", span c12862a73454ecc1                                      
    │   ├── Kind : CLIENT                                                                                                           
    │   └── Attributes :                                                                                                            
    │       ├── db.system : sqlite                                                                                                  
    │       ├── db.name : sample.db                                                                                                 
    │       ├── db.statement :                                                                                                      
    │       └── SELECT "id","name","private" FROM "locations"                                                                       
    ├── [01:40:55.145724] /locations http send, span 08dff3718cf0a4a3                                                               
    │   ├── Kind : INTERNAL                                                                                                         
    │   └── Attributes :                                                                                                            
    │       ├── http.status_code : 200                                                                                              
    │       └── type : http.response.start                                                                                          
    └── [01:40:55.147593] /locations http send, span d3088e87ff397dc4                                                               
        ├── Kind : INTERNAL                                                                                                         
        └── Attributes :                                                                                                            
            └── type : http.response.body                                 

Colorised:

screenshot 2021-09-18 at 13 16 51

@tonybaloney tonybaloney requested a review from a team September 19, 2021 22:55
@owais
Copy link
Contributor

owais commented Sep 19, 2021

Thanks. This looks very nice and useful but is this something that the OpenTelemetry project should host and maintain? This could easily be hosted independently and published as a 3rd party package to pypi.org. Curious what other @open-telemetry/python-approvers @open-telemetry/python-maintainers think about it.

@owais
Copy link
Contributor

owais commented Sep 19, 2021

If we were to host it in this repo, I think it'd be better to add the feature to the existing console exporter and make it configurable to print either format.

@tonybaloney
Copy link
Contributor Author

If we were to host it in this repo, I think it'd be better to add the feature to the existing console exporter and make it configurable to print either format.

Happy to host + publish this one myself. Can I use the same package name (opentelemetry-exporter-richconsole) ?

If it were added to the console exporter, you'd have to add Rich as a dependency to the console exporter, which would be overkill IMO

@srikanthccv
Copy link
Member

If it were added to the console exporter, you'd have to add Rich as a dependency to the console exporter, which would be overkill IMO

Making it optional dependency with extras_require and use rich format if it exists?

@owais
Copy link
Contributor

owais commented Sep 20, 2021

Can I use the same package name (opentelemetry-exporter-richconsole) ?

I think so. Unfortunately pypi doesn't have namespaces like NPM and I don't think we can enforce opentelemetry- prefix not being used by others so eventually packages will pop up with that prefix. I think it's fine.

@aabmass
Copy link
Member

aabmass commented Sep 22, 2021

This looks very nice and useful but is this something that the OpenTelemetry project should host and maintain?

@owais how does this differ from other exporters/instrumentations in this repo? Trying to understand arguments for and against this being merged here.

@owais
Copy link
Contributor

owais commented Sep 22, 2021

@aabmass I'm not sure either and that is why I'm bringing it up. I don't think there is anything inherently wrong with it being in contrib but at the same time every new package adds additional burden on maintainers and systems (CI) both. It is usually worth it for packages that improve the quality of telemetry this project can provide (instrumentations) but not sure if every dev/debugging utility needs to be hosted in this repo. May be we can encourage the broader OpenTelemetry (Python) ecosystem to grow beyond core and contrib repos. I don't feel strongly either way but felt it was an interesting this to talk about.

@aabmass
Copy link
Member

aabmass commented Sep 23, 2021

@owais I think this one adds a lot of value for developers (both instrumentation authors and application developers). I'm in favor of adding this, but I agree it's worth a general discussion. I've added this PR to the agenda for next week's SIG.

@owais
Copy link
Contributor

owais commented Sep 27, 2021

@tonybaloney please fix lint issues, add a changelog entry and we can merge this.

Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

A few comments, otherwise LGTM

tonybaloney and others added 3 commits September 28, 2021 07:09
Copy link
Member

@srikanthccv srikanthccv left a comment

Choose a reason for hiding this comment

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

LGTM. Added couple of comments.

@tonybaloney
Copy link
Contributor Author

Whats the correct way to run the linter to get the rest of this to pass?

I tried running black but it reformats everything, tried scripts/eachdist.py lint and that reformatted everything as well

@owais
Copy link
Contributor

owais commented Sep 27, 2021

@tonybaloney make sure your installed black and isort versions are the same as pinned in dev-requirements.txt, then run ./scripts/eachdist.py format

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Just a couple of comments, this is a good debugging tool!

@tonybaloney
Copy link
Contributor Author

@codeboten done and done!

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments!

@codeboten
Copy link
Contributor

@aabmass please approve if your comments have been addressed

Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

🚢

@ocelotl ocelotl enabled auto-merge (squash) September 29, 2021 14:10
@ocelotl ocelotl merged commit bba4b9e into open-telemetry:main Sep 29, 2021
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.

6 participants