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 Apolloconfig Inst... #12787

Open
freshchen opened this issue Nov 25, 2024 · 9 comments
Open

Add Apolloconfig Inst... #12787

freshchen opened this issue Nov 25, 2024 · 9 comments
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request new instrumentation

Comments

@freshchen
Copy link

Is your feature request related to a problem? Please describe.

https://github.com/apolloconfig/apollo is a cloud native configuration management system。Apolloconfig uses http long connection to update configuration in real time. And the realtime update support gray.

When we meet strange problems, you need to check whether the configuration has been sent to the corresponding POD.

Describe the solution you'd like

Is it possible to add an apolloconfig inst to intercept the onchage method?

What I don't know is whether there is semantic for configuration updates.

Describe alternatives you've considered

No response

Additional context

No response

@freshchen freshchen added enhancement New feature or request needs triage New issue that requires triage labels Nov 25, 2024
@laurit
Copy link
Contributor

laurit commented Nov 25, 2024

We'd welcome a contribution for this.

What I don't know is whether there is semantic for configuration updates.

My guess would be that there are no semantic conventions for this, contributors are welcome to work on this in the the semantic conventions repository.

@laurit laurit added contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome new instrumentation and removed needs triage New issue that requires triage labels Nov 25, 2024
@freshchen
Copy link
Author

Ok, I have time to do this work recently, but it is my first contribution. Is there any slack that can provide help, such as tests, docs, etc.

@trask
Copy link
Member

trask commented Nov 25, 2024

hi @freshchen, you can find the Java Slack channel here: https://github.com/open-telemetry/community#implementation-sigs

another option also is to contribute OpenTelemetry instrumentation directly to the Apolloconfig project

freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 26, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 26, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 26, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 26, 2024
@freshchen
Copy link
Author

@trask Thx for post the link。I have completed my first instrumentation PR. The whole process was very smooth. The CI & Doc of this project is cool.

freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 27, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 28, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 29, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Nov 29, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 3, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 4, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 4, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 4, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 4, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 6, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 6, 2024
freshchen added a commit to freshchen/opentelemetry-java-instrumentation that referenced this issue Dec 6, 2024
@freshchen
Copy link
Author

freshchen commented Dec 10, 2024

During the code review, there were quite a few discussions. I think some gaps exist.

Below, I’ll provide some additional explanations, hoping to help you define this instrumentation.

1 Expect

1.1 demo

Modifying the configuration on the apollo web page will not only update the spring environment in real time。

I add some logic through the @ApolloConfigChangeListener annotation. For example, when changing the oauth clientId & clientSecret of a third party, we expect to first verify that the new configuration meets the business needs, and then replace the value in the program.

@ApolloConfigChangeListener  
private void onChange(ConfigChangeEvent changeEvent) {  
  
    log.info("try to change xxx service clientId & clientSecret");  
  
    oauthClient.getToken();  
    oauthClient.checkTokenScope();  
  
    log.info("new clientId & clientSecret enabled");  
}


# Two fake endpoint

@GetMapping("/oauth/token")  
public RestResponse<String> token() {  
    log.info("get oauth token by ak*****sd / sa*******ada");  
    return RestResponse.success("test");  
}  
  
@GetMapping("/oauth/token/scopeCheck")  
public RestResponse<String> tokenTest() {  
    log.info("the new token contains all required scopes");  
    return RestResponse.success("test");  
}

# and client

@RestClient("OauthClient")  
public interface OauthClient {  
  
    @GetExchange("/oauth/token")  
    RestResponse<String> getToken();  
  
    @GetExchange("/oauth/token/scopeCheck")  
    RestResponse<String> checkTokenScope();  
}

1.2 expect observable signal

The observable signal I expect is as follows

For Trace:

image

For Log:

2024-12-10 14:19:59,717 [tid=f67725b39fba211179de95d38f3a0fbb sid=f5ceaf722f6a42c3] [Apollo-Config-2][INFO] c.b.mapcloud.one.javaagent.test.App.? - try to change xxx service clientId & clientSecret

2024-12-10 14:19:59,719 [tid=f67725b39fba211179de95d38f3a0fbb sid=f5ceaf722f6a42c3] [Apollo-Config-1][INFO] c.c.f.a.s.p.AutoUpdateConfigChangeListener.? - Auto update apollo changed value successfully, new value: adas*****2, key: service.xxx.oauth.client-secret

2024-12-10 14:19:59,742 [tid=f67725b39fba211179de95d38f3a0fbb sid=35cf128c9b42c215] [Apollo-Config-2][INFO] c.b.m.c.r.c.l.RestClientLoggerInterceptor.? - RestClient Req  method=GET ; url=http://127.0.0.1:8080/oauth/token

2024-12-10 14:19:59,849 [tid=f67725b39fba211179de95d38f3a0fbb sid=f1dbf08e36362d52] [tomcat-handler-0][INFO] c.b.m.cloudnative.web.log.HttpLog.? - Req method=GET ; url=http://127.0.0.1:8080/oauth/token

2024-12-10 14:19:59,867 [tid=f67725b39fba211179de95d38f3a0fbb sid=f1dbf08e36362d52] [tomcat-handler-0][INFO] c.b.m.o.j.t.c.TestController.? - get oauth token by ak*****sd / sa*******ada

2024-12-10 14:19:59,906 [tid=f67725b39fba211179de95d38f3a0fbb sid=f1dbf08e36362d52] [tomcat-handler-0][INFO] c.b.m.cloudnative.web.log.HttpLog.? - Resp httpCode=200 ; body={"success":true,"code":0,"msg":"ok","data":"test"} ; cost=total:57ms

2024-12-10 14:19:59,942 [tid=f67725b39fba211179de95d38f3a0fbb sid=9d55bfb630453d91] [Apollo-Config-2][INFO] c.b.m.c.r.c.l.RestClientLoggerInterceptor.? - RestClient Req  method=GET ; url=http://127.0.0.1:8080/oauth/token/scopeCheck

2024-12-10 14:19:59,943 [tid=f67725b39fba211179de95d38f3a0fbb sid=c8078cfb93f7742f] [tomcat-handler-1][INFO] c.b.m.cloudnative.web.log.HttpLog.? - Req method=GET ; url=http://127.0.0.1:8080/oauth/token/scopeCheck

2024-12-10 14:19:59,944 [tid=f67725b39fba211179de95d38f3a0fbb sid=c8078cfb93f7742f] [tomcat-handler-1][INFO] c.b.m.o.j.t.c.TestController.? - the new token contains all required scopes

2024-12-10 14:19:59,944 [tid=f67725b39fba211179de95d38f3a0fbb sid=c8078cfb93f7742f] [tomcat-handler-1][INFO] c.b.m.cloudnative.web.log.HttpLog.? - Resp httpCode=200 ; body={"success":true,"code":0,"msg":"ok","data":"test"} ; cost=total:1ms

2024-12-10 14:19:59,946 [tid=f67725b39fba211179de95d38f3a0fbb sid=f5ceaf722f6a42c3] [Apollo-Config-2][INFO] c.b.mapcloud.one.javaagent.test.App.? - new clientId & clientSecret enabled

2 discuss

2.1 About Signal Type

First of all, through the above demo, I think this should not be a 0 during event

2.2 About Span Attribute

The following figure is the web page of Apollo. You can see that it splits and reuses the configuration namespace granularity. The minimum granularity of configuration release is namespace.

image

In order to avoid involving sensitive information and reduce the storage cost of span, I think it is not necessary to put the changed configuration items into the span.

2.3 About How Apollo client work

Finally, I will provide some of my personal understanding of the configuration changes of this project.

image

@steverao
Copy link
Contributor

We have a biweekly meeting that is friendly for Asian developers in time. If you have time, you can join us to discuss it tomorrow morning. This is meeting hyperlink: https://zoom.us/j/97160857165?pwd=N3Bwc0s1MVVJWjZkK0E0QXJWSWhoZz09
image

@123liuziming
Copy link
Contributor

123liuziming commented Dec 12, 2024

Should we include more metrics about the config/registry center including:

  1. subscribed services count
  2. published services count
  3. listened configuration file count
  4. request count
  5. request time

Theses metrics may be more useful to users compared to a single span.

@freshchen
Copy link
Author

We have a biweekly meeting that is friendly for Asian developers in time. If you have time, you can join us to discuss it tomorrow morning. This is meeting hyperlink: https://zoom.us/j/97160857165?pwd=N3Bwc0s1MVVJWjZkK0E0QXJWSWhoZz09 image

Sorry,Sorry, I just saw this, is there any conclusion to the meeting?

@freshchen
Copy link
Author

Should we include more metrics about the config/registry center including:

  1. subscribed services count
  2. published services count
  3. listened configuration file count
  4. request count
  5. request time

Theses metrics may be more useful to users compared to a single span.

Sure, it is necessary, I think this work the same applies to nacos, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request new instrumentation
Projects
None yet
Development

No branches or pull requests

5 participants