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

Baggage value is not parsed correctly in downstream server #2010

Closed
ashu658 opened this issue Jul 30, 2021 · 4 comments · Fixed by #2167
Closed

Baggage value is not parsed correctly in downstream server #2010

ashu658 opened this issue Jul 30, 2021 · 4 comments · Fixed by #2167
Assignees
Labels
bug Something isn't working good first issue Good first issue help wanted

Comments

@ashu658
Copy link
Member

ashu658 commented Jul 30, 2021

Environment:
Python 3.7.9, Ubuntu 20.04
opentelemetry-api 1.4.0
opentelemetry-sdk 1.4.0
flask 1.1.3
requests 2.26.0

Setup: A flask upstream server (to set_baggage) and downstream server (to get_baggage).
Attached requirements.txt if any additional dependency information is needed

Steps to reproduce
Upon setting the baggage in the upstream server and trying to get baggage in downstream server, I am not able to get the original value of the baggage.
Whitespaces are replaced by '+' symbol. e.g. Value in baggage My Random Value changes to My+Random+Value

In the upstream server I am able to extract the original value of the baggage.
Sample code for upstream

# UPSTREAM SERVER
from opentelemetry.context import get_current
from opentelemetry.baggage import set_baggage, get_all
from opentelemetry.propagate import inject, extract
from opentelemetry.baggage import get_baggage
@app.route('/upstream')
def hello_world():
    current_context = get_current()
    current_context = set_baggage('My key', 'My Random Value', current_context)
    headers = {}
    inject(headers, context=current_context)
    print(get_baggage('My key', current_context))
    # output of print is: 'My Random Value'
    r = requests.get('http://localhost:9023/downstream', headers=headers)
    return r.content

But in downstream server when I extract the baggage using get_baggage I am not able to get the original value.

# DOWNSTREAM SERVER
from flask import request
from opentelemetry.propagate import inject, extract
from opentelemetry.baggage import get_baggage
import opentelemetry.instrumentation.wsgi as otel_wsgi
@app.route('/downstream')
def hello_world():
    extracted_context = extract(request.environ, getter=otel_wsgi.wsgi_getter)
    print(get_baggage('My key', extracted_context))
    # output of print is: 'My+Random+Value'
    return 'Called downstream server!'

Expected behaviour
Downstream server should be able to get original value of baggage.

@ashu658 ashu658 added the bug Something isn't working label Jul 30, 2021
@owais
Copy link
Contributor

owais commented Jul 30, 2021

Issue is that we use quote_plus to encode but quote to decode.


We need to figure out which encoding the spec recommends and other otel projects use before fixing this.

@srikanthccv
Copy link
Member

https://w3c.github.io/baggage/#definition

baggage-string         =  list-member 0*179( OWS "," OWS list-member )
list-member            =  key OWS "=" OWS value *( OWS ";" OWS property )
property               =  key OWS "=" OWS value
property               =/ key OWS
key                    =  token ; as defined in RFC 2616, Section 2.2
value                  =  *baggage-octet
baggage-octet          =  %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                          ; US-ASCII characters excluding CTLs,
                          ; whitespace, DQUOTE, comma, semicolon,
                          ; and backslash
OWS                    =  *( SP / HTAB ) ; optional white space, as defined in RFC 7230, Section 3.2.3

By this definition baggage values with space shouldn't be allowed in the first place, isn't it?

@srikanthccv
Copy link
Member

ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 3, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 3, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 3, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 5, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 13, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 17, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 27, 2021
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Aug 27, 2021
@github-actions
Copy link

This issue was marked stale due to lack of activity. It will be closed in 30 days.

ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Sep 9, 2021
@ocelotl ocelotl assigned srikanthccv and unassigned ocelotl Sep 30, 2021
owais pushed a commit that referenced this issue Oct 13, 2021
* Add regexes to check keys and values

Fixes #2010

* Fix tests

* Add changelog

* Fix test

* Add checks to set_baggage

* Fix lint

* Add changelog entry

* Fix mypy

* Fix mypy

* Cast to string

* WIP

* Key value format

* Mostly done

* Remove old changelog entry

* fomat

* Correct typing

* Fix lint

* Fix issues

* Add CHANGELOG entry

* Make changes as discussed in SIG meeting

* Update opentelemetry-api/src/opentelemetry/baggage/__init__.py

Co-authored-by: Leighton Chen <[email protected]>

Co-authored-by: Diego Hurtado <[email protected]>
Co-authored-by: Leighton Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good first issue help wanted
Projects
None yet
4 participants