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

use innerEvent to encapsulate user data only when the tracing is enabled #49

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Conversation

tpiperatgod
Copy link
Member

@tpiperatgod tpiperatgod commented Jun 7, 2022

this PR also supports allowing the user to decide whether to use the raw data to send.

example:

func Pub(ctx ofctx.Context, in []byte) (ofctx.Out, error) {
	msg := map[string]string{
		"hello": "world",
	}

	msgBytes, _ := json.Marshal(msg)

	// This setting allows Send() to send the raw user data, even when the Tracing is enabled
	ctx.ContextOptions().SetRawData(true)

	res, err := ctx.Send("subscriber", msgBytes)
	if err != nil {
		klog.Error(err)
		return ctx.ReturnOnInternalError(), err
	}
	klog.Infof("send msg and receive result: %s", string(res))

	return ctx.ReturnOnSuccess(), nil
}

Signed-off-by: laminar fangtian@kubesphere.io

@@ -363,7 +372,7 @@ func (ctx *FunctionContext) Send(outputName string, data []byte) ([]byte, error)

payload = data

if traceable(output.ComponentType) {
if isTracingEnabled(ctx) && traceable(output.ComponentType) && !ctx.GetOptions().GetSendWithRawData() {
Copy link
Member

Choose a reason for hiding this comment

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

What happens if tracing is enabled and the user chooses to send with raw data?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, let me define the priority as follows:

the user decides whether to send raw data > tracing is enabled > target is traceable

Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

I think the logic is added specially for SkyWalking. Can we replace the !GetSendWithRawData with something like IsTracingProviderSkyWalking?

If user enables skywalking tracing, it has to be cloud events, else keep the original data.
Does this make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

context/context.go Outdated Show resolved Hide resolved
this also supports allowing the user to decide whether to use the raw data to send

Signed-off-by: laminar <fangtian@kubesphere.io>
@tpiperatgod
Copy link
Member Author

innerEvent will be enabled when all of the following conditions are met:

  • Tracing is enabled
  • Tracing provider is "skywalking"
  • The output target is a traceable component
  • RawData is set to false(default) in ContextOption

@lizzzcai
Copy link
Member

innerEvent will be enabled when all of the following conditions are met:

  • Tracing is enabled
  • Tracing provider is "skywalking"
  • The output target is a traceable component
  • RawData is set to false(default) in ContextOption

If skywalking is enabled for tracing but innerEvent is disabled (for example RawData is set to true), what will happen to the skywalking tracing?

@benjaminhuo
Copy link
Member

That's a nice move.
We have metadata added by introducing options.

@benjaminhuo benjaminhuo merged commit 4e00aaf into OpenFunction:main Jun 13, 2022
@benjaminhuo
Copy link
Member

benjaminhuo commented Jun 13, 2022

innerEvent will be enabled when all of the following conditions are met:

  • Tracing is enabled
  • Tracing provider is "skywalking"
  • The output target is a traceable component
  • RawData is set to false(default) in ContextOption

If skywalking is enabled for tracing but innerEvent is disabled (for example RawData is set to true), what will happen to the skywalking tracing?

The sync function tracing still works while the async function tracing doesn't, is that right?

@tpiperatgod
Copy link
Member Author

If skywalking is enabled for tracing but innerEvent is disabled (for example RawData is set to true), what will happen to the skywalking tracing?

the tracing will be broken in this case.

@tpiperatgod tpiperatgod deleted the dev branch June 13, 2022 15:27
@tpiperatgod
Copy link
Member Author

The sync function tracing still works while the async function tracing doesn't, is that right?

yes, the sync function can carry the tracing metadata through HTTP headers.

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.

None yet

3 participants