Conversation
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
…andard Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Richard Salac <richard.salac@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
…andard Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
apiml/src/test/java/org/zowe/apiml/acceptance/OpenTelemetryMetricsTests.java
Outdated
Show resolved
Hide resolved
apiml/src/test/java/org/zowe/apiml/acceptance/OpenTelemetryMetricsTests.java
Outdated
Show resolved
Hide resolved
| assertEquals("zos", attributes.get(stringKey("os.type"))); | ||
| assertEquals("STC1111", attributes.get(stringKey("process.zos.jobid"))); | ||
| assertEquals("ZWE1AG", attributes.get(stringKey("process.zos.jobname"))); | ||
| assertEquals("gateway", attributes.get(stringKey("service.name"))); | ||
| assertEquals("apiml:apiml1:40985", attributes.get(stringKey("service.namespace"))); | ||
| assertNotNull(attributes.get(stringKey("service.version"))); |
There was a problem hiding this comment.
The os.name is set before but not validated.
There was a problem hiding this comment.
What do you mean? This validates the attributes we set, os.name is part of the default implementation. If we want to test those I'd add them to the default integration test, otherwise we'd be testing a feature of the core library
There was a problem hiding this comment.
On line 60 the os.name is set in the static initializer (and then again in BeforeAll on line 68), and populated to resource attributes on line 123. But never asserted. Hence you assert os.type. I thought this is to test overrides - what happens if we set a property automatically discovered by the OpenTelemetry SDK
There was a problem hiding this comment.
os.name is set simply to force the use of the Zos spring bean. In this case sure, I can set any otel. resource property and assert it. I'll add one or two to cover for it, but we are basically using Spring's default for those
There was a problem hiding this comment.
You are absolutely right and I missed that it is set to trigger the z/os attributes provider.
apiml/src/test/java/org/zowe/apiml/acceptance/OpenTelemetryMetricsTests.java
Outdated
Show resolved
Hide resolved
apiml-common/src/main/java/org/zowe/apiml/product/opentelemetry/ZosAttributes.java
Show resolved
Hide resolved
| } | ||
|
|
||
| public Map<String, Object> get() { | ||
| return Map.of( |
There was a problem hiding this comment.
Can we somehow get the address space id?
There was a problem hiding this comment.
Not with this interface, needs research
There was a problem hiding this comment.
any progress? Does maybe launcher provides it? If not just leave a note it and close the conversation
There was a problem hiding this comment.
not really, I don't see in the launcher code that refers to the address space ID. It's only using USS environment variables to control some address space related things.
We should create a work item to follow up on this if we deem it important.
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
Signed-off-by: Pablo Carle <pablo.carle@broadcom.com>
|
| } | ||
|
|
||
| public Map<String, Object> get() { | ||
| return Map.of( |
There was a problem hiding this comment.
any progress? Does maybe launcher provides it? If not just leave a note it and close the conversation
| private String generateServiceNamespace(Map<String,Object> zosAttributes) { | ||
| return "apiml:" + generateServiceName(zosAttributes); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think it makes sense when it is only service name with a prefix. Then it has no additional value over the service name in therm of identifying the installation.
| Optional.ofNullable(zosAttributes.get(ZOS_USER_ID)).map(String::valueOf) | ||
| .ifPresent(zosUserId -> attributesBuilder.put("process.zos.userid", zosUserId)); | ||
|
|
||
| return attributesBuilder.build(); |
There was a problem hiding this comment.
Ok, I got the impression that you prefer the lpar+port over the uuid.
| private final ZosSystemInformation zosSystemInformation; | ||
|
|
||
| @Value("${otel.resource.attributes.deployment.environment.name:#{null}}") | ||
| private String environmentName; |
There was a problem hiding this comment.
I think we should remove it form here.
| Optional.ofNullable(zosAttributes.get(ZOS_JOB_ID)) | ||
| .map(String::valueOf) | ||
| .filter(StringUtils::isNotBlank) | ||
| .ifPresent(zosJobId -> attributesBuilder.put(ZosAttributes.ZOS_JOBID, zosJobId)); |
There was a problem hiding this comment.
The constant naming is confusing: ZOS_JOB_ID vs ZOS_JOBID
and similarly for some others
There was a problem hiding this comment.
I can rename them, but they are different values anyway, one is zos.jobid from the system information and the other one is process.zos.jobid for the target OpenTelemetry attribute
There was a problem hiding this comment.
It is confusing, as you say they mean something totally different and yet named so similarly. Maybe "OTEL_ZOS_JOBNAME" or "OtelZosAttributes.ZOS_JOBNAME"?
...rc/main/java/org/zowe/apiml/product/opentelemetry/ApimlZosOpenTelemetryResourceProvider.java
Show resolved
Hide resolved
| if (StringUtils.isBlank(sysplexName)) { | ||
| var sysplexName = zosAttributes.get(ZOS_SYSPLEX); | ||
| if (sysplexName != null && StringUtils.isNotBlank(sysplexName.toString())) { | ||
| log.debug("zos.sysplex.name not provided in configuration, using system-obtained {}", sysplexName); | ||
| } else { | ||
| log.debug("zos.sysplex.name not provided in configuration. Could not determine name from system"); | ||
| } | ||
| } | ||
|
|
||
| if (StringUtils.isBlank(lparName)) { | ||
| var lparName = zosAttributes.get(ZOS_SYSNAME); | ||
| if (lparName != null && StringUtils.isNotBlank(lparName.toString())) { | ||
| log.debug("mainframe.lpar.name not provided in configuration, using system-obtained {}", lparName); | ||
| } else { | ||
| log.debug("mainframe.lpar.name not provided in configuration. Could not determine name from system"); | ||
| } | ||
| } | ||
|
|
||
| if (StringUtils.isBlank(smfId)) { | ||
| var smfId = zosAttributes.get(ZOS_SYSCLONE); | ||
| if (smfId != null && StringUtils.isNotBlank(smfId.toString())) { | ||
| log.debug("zos.smf.id not provided in configuration, using system-obtained {}", smfId); | ||
| } else { | ||
| log.debug("zos.smf.id not provided in configuration. Could not determine ID from system"); | ||
| } | ||
| } |
There was a problem hiding this comment.
They are checked for value but never set to the attributes.
There was a problem hiding this comment.
I added these only to show in the logs if such overrides are happening, if there's no value in the logs we can remove it. We don't need to explicitly set them because they are otel.* properties which are automatically picked up by OpenTelemetry implementation.
There was a problem hiding this comment.
Regardless it is configured or discovered, they are not set in resource attributes and are not present in the exported data.


Description
Implementation of required / additional / recommended properties for base signals from APIML OpenTelemetry implementation with defaults.
Linked to # (issue)
Part of the # (epic)
Type of change
Checklist: