kezhenxu94 commented on a change in pull request #55:
URL: https://github.com/apache/skywalking-python/pull/55#discussion_r463918596
##########
File path: skywalking/trace/context/__init__.py
##########
@@ -118,23 +119,44 @@ def active_span(self):
return None
+ def get_correlation(self, key):
+ if key in self._correlation:
+ return self._correlation[key]
+ return None
+
+ def put_correlation(self, key, value):
+ if key is None or value is None:
Review comment:
If the `value is None`, I'm expecting the users want to remove the item,
but you just ignore it
##########
File path: tests/plugin/sw_flask/services/provider.py
##########
@@ -24,12 +24,16 @@
config.logging_level = 'DEBUG'
agent.start()
- from flask import Flask, jsonify
+ from flask import Flask, jsonify, request
app = Flask(__name__)
@app.route("/users", methods=["POST", "GET"])
def application():
+ print(request.headers)
+ from skywalking.trace.context import get_context
+ print(get_context().get_correlation("test"))
+ print(get_context().get_correlation("test2"))
Review comment:
Do not simply print the correlation headers, this can not verify
anything, even the headers are empty, the tests still passed, one way is to
propagate these headers to the test codes, return the headers as the http
response, for example, then get the response and verify the expected data here,
`self.validate()` returns the response, and you can get the response and verify
further
https://github.com/apache/skywalking-python/blob/0246634b8e908b845e1540c6d2b70d9058794601/tests/plugin/sw_flask/test_flask.py#L38
##########
File path: skywalking/trace/span/__init__.py
##########
@@ -225,5 +226,10 @@ class NoopSpan(Span):
def __init__(self, context: 'SpanContext' = None, kind: 'Kind' = None):
Span.__init__(self, context=context, kind=kind)
+ def extract(self, carrier: 'Carrier') -> 'Span':
+ if carrier is not None:
+ self.context._correlation = carrier.correlation_carrier.correlation
+
def inject(self, carrier: 'Carrier') -> 'Span':
+ carrier.correlation_carrier.correlation = self.context._correlation
Review comment:
What about making `_correlation` public or expose a property for it,
instead of accessing a protected member directly?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
