-
Notifications
You must be signed in to change notification settings - Fork 20
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 tracing configuration to the Context #26
Conversation
Signed-off-by: laminar <fangtian@kubesphere.io>
This reverts commit c288157.
return nil, fmt.Errorf("invalid tracing provider name: %s", ctx.PluginsTracing.Provider.Name) | ||
} | ||
if ctx.PluginsTracing.Tags != nil { | ||
if funcName, ok := ctx.PluginsTracing.Tags["func"]; !ok || funcName != ctx.Name { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might need to add pod's name to tracing plugin's tags map like replica: <podname>
, together with func: <functionname>
, it will provide more accurate tracing data. Simply add func: <functionname>
is not ok, because a function has many replicas and the tracing should be specific to replica. Maybe the `replica: should be added in the tracing plugin code, but can we provide a function somewhere to retrieve current function replica's pod name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the pod's name can be passed to the container via env.
like this: https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/#use-pod-fields-as-values-for-environment-variables ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's what I mean.
And we might need to add a function to be called by the tracing plugin maybe into the function framework?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The namespace is also useful to be added to tags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added parsing function for the POD_NAME
and POD_NAMESPACE
environment variables, which have the following association with tags
in the Tracing configuration:
tags['replica'] = <value from POD_NAME>
tags['namespace'] = <value from POD_NAMESPACE>
I wonder if this could solve the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in discussion OpenFunction/functions-framework#9 (comment), skywalking has service
, instance
and endpoint
concept, for now, it seems that function is a service
, replica
is an instance
, can we change replica
to instance
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added parsing function for the
POD_NAME
andPOD_NAMESPACE
environment variables, which have the following association withtags
in the Tracing configuration:tags['replica'] = <value from POD_NAME> tags['namespace'] = <value from POD_NAMESPACE>
I wonder if this could solve the problem?
Yeah, it solved the problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has changed replica
to instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return nil, fmt.Errorf("invalid tracing provider name: %s", ctx.PluginsTracing.Provider.Name) | ||
} | ||
if ctx.PluginsTracing.Tags != nil { | ||
if funcName, ok := ctx.PluginsTracing.Tags["func"]; !ok || funcName != ctx.Name { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in discussion OpenFunction/functions-framework#9 (comment), skywalking has service
, instance
and endpoint
concept, for now, it seems that function is a service
, replica
is an instance
, can we change replica
to instance
?
Signed-off-by: laminar <fangtian@kubesphere.io>
By default, the Tracing plugin is registered at the end of
prePlugins
and at the beginning ofpostPlugins